Properly deduplicate connections.

By assigning a random connection ID during the initial hello that's both 
unique to every new connection regardless of target and consistent 
across both the connection initiator and acceptor, we can do string 
comparison to determine which string is greater than the other. In doing 
so, we can ensur  the same connection is always terminated, thereby 
avoiding closing both connections by accident.

src/lib/agent/Peer.mjs

@@ -5,6 +5,7 @@ import { EventEmitter, once } from 'events';
 import log from '../io/NamespacedLog.mjs'; const l = log("peer");
 
 import Connection from '../transport/Connection.mjs';
+import make_crypto_id from '../crypto/make_crypto_id.mjs';
 
 class Peer extends EventEmitter {
 	get address() { return this.connection.address; }
@@ -36,6 +37,15 @@ class Peer extends EventEmitter {
 		 * @type {string}
 		 */
 		this.id = null;
+		/**
+		 * The ID of this connection.
+		 * Every time a connection is opened, a new connection ID is negotiated
+		 * in the initial hello handshake.
+		 * The connection ID is consistent across both the connection initiator
+		 * and acceptor.
+		 * @type {string?}
+		 */
+		this.conn_id = null;
 		/**
 		 * The friendly name of this peer.
 		 * Unlike the ID, this *may* not be unique (though it is strongly
@@ -70,7 +80,7 @@ class Peer extends EventEmitter {
 	async __accept(connection) {
 		this.connection = connection;
 		const [ msg ] = await once(this.connection, "message-hello");
-		if(!this.__handle_hello(msg))
+		if(!this.__handle_hello(msg, false))
 			await this.destroy();
 		
 		await this.__send_hello();
@@ -85,27 +95,31 @@ class Peer extends EventEmitter {
 		return new Promise((resolve, reject) => {
 			this.connection.once("error", reject);
 			this.connection.once("message-hello", async (msg) => {
-				if(!this.__handle_hello(msg))
+				if(!this.__handle_hello(msg, true))
 					await this.destroy();
 				
 				this.connection.off("error", reject);
 				this.emit("connect");
 				resolve();
 			});
+			this.conn_id = make_crypto_id();
 			this.__send_hello(); // Set and forget
 		});
 	}
 	
 	/**
 	 * Handles a given hello messaage from this peer.
-	 * @param	{Object}	msg		The hello message to process.
+	 * @param	{Object}	msg			The hello message to process.
+	 * @param	{boolean}	initiator	Whether we were the initiator of this connection or not.
 	 * @return	{boolean}	Whether the peer should stay connected or not.
 	 */
-	__handle_hello(msg) {
+	__handle_hello(msg, initiator) {
 		this.id = msg.id;
 		this.name = msg.name;
 		this.listening_address = msg.listening_address;
 		this.listening_port = msg.listening_port;
+		if(!initiator)
+			this.conn_id = msg.conn_id;
 		
 		if(this.id === this.server.our_id) {
 			l.warn(`Our id (${this.server.our_id}) is equal to that of the remote (${this.id}), killing connection`);
@@ -128,6 +142,7 @@ class Peer extends EventEmitter {
 	async __send_hello() {
 		await this.send("hello", {
 			id: this.server.our_id,
+			conn_id: this.conn_id,
 			name: this.server.our_name,
 			listening_address: this.server.host,
 			listening_port: this.server.port

src/lib/agent/PeerServer.mjs

@@ -243,18 +243,38 @@ class PeerServer extends EventEmitter {
 		if(peer === null) return null;
 		// If the ID of the peer is equal to that of one of the existing peers, close the
 		// connection
-		if(this.peers().some(existing_peer => existing_peer.id === peer.id)) {
+		for(const existing_peer of this.peers()) {
-			l.info(`Closing duplicate connection to ${peer.id_short}`);
+			if(existing_peer.id === peer.id) {
-			await this.remove_peers(peer);
+				// If this peer was reaped, don't continue
+				if(this.dedupe(peer, existing_peer) === peer)
+					return null;
+			}
 		}
-		if(this.connecting_peers.filter(other_peer => other_peer.id !== null)
-			.some(other_peer => other_peer.id === peer.id))
-			await this.remove_peers(peer);
 		
 		this.peer_initialise(peer);
 		return peer;
 	}
 	
+	/**
+	 * Deduplicates 2 idential peers.
+	 * This function ensures that deduplication does *not* result in 2 closed
+	 * connections, and that 1 connection to the peer in question remains open.
+	 * @param  {Peer}  peer_a	The first duplicate peer.
+	 * @param  {Peer}  peer_b	The second duplicate peer.
+	 * @return {Promise<bool>}	A Promise that resolves when deduplication is complete. Returns the reaped peer.
+	 */
+	async dedupe(peer_a, peer_b) {
+		if(peer_a.id !== peer_b.id) return null;
+		if(peer_a.conn_id > peer_b.conn_id) {
+			await this.remove_peers(peer_b);
+			return peer_b;
+		}
+		else {
+			await this.remove_peers(peer_a);
+			return peer_a;
+		}
+	}
+	
 	async remove_peers(...peers) {
 		// No need to remove the peer from the list of connected peers, as
 		// there's already a catch-all event handler attached to do that

src/lib/crypto/make_crypto_id.mjs

@@ -0,0 +1,10 @@
+"use strict";
+
+import tweetnacl from 'tweetnacl';
+
+
+export default function make_crypto_id() {
+	return Buffer.from(tweetnacl.randomBytes(256)).toString("base64")
+		.replace(/\+/g, "-")
+		.replace(/\//g, "_");
+}