From 8f6600facf838a3155b552a4fe7d08a24e096922 Mon Sep 17 00:00:00 2001
From: Stephen Birarda <commit@birarda.com>
Date: Tue, 22 May 2018 14:14:36 -0700
Subject: [PATCH] remove ICE peers once nodes connect or are killed

---
 domain-server/src/DomainGatekeeper.cpp   | 13 +++++++++++++
 domain-server/src/DomainGatekeeper.h     |  8 ++++++--
 domain-server/src/DomainServer.cpp       | 11 +++++++++--
 domain-server/src/DomainServerNodeData.h |  7 ++++++-
 4 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/domain-server/src/DomainGatekeeper.cpp b/domain-server/src/DomainGatekeeper.cpp
index 8e5df1a6f1..47b55bb5c2 100644
--- a/domain-server/src/DomainGatekeeper.cpp
+++ b/domain-server/src/DomainGatekeeper.cpp
@@ -520,6 +520,10 @@ SharedNodePointer DomainGatekeeper::addVerifiedNodeFromConnectRequest(const Node
     // create a new node ID for the verified connecting node
     auto nodeID = QUuid::createUuid();
 
+    // add a mapping from connection node ID to ICE peer ID
+    // so that we can remove the ICE peer once we see this node connect
+    _nodeToICEPeerIDs.insert(nodeID, nodeConnection.connectUUID);
+
     auto limitedNodeList = DependencyManager::get<LimitedNodeList>();
 
     Node::LocalID newLocalID = findOrCreateLocalID(nodeID);
@@ -533,6 +537,15 @@ SharedNodePointer DomainGatekeeper::addVerifiedNodeFromConnectRequest(const Node
     return newNode;
 }
 
+void DomainGatekeeper::cleanupICEPeerForNode(const QUuid& nodeID) {
+    // remove this node ID from our node to ICE peer ID map
+    // and the associated ICE peer (if it still exists)
+    auto icePeerID = _nodeToICEPeerIDs.take(nodeID);
+    if (!icePeerID.isNull()) {
+        _icePeers.remove(icePeerID);
+    }
+}
+
 bool DomainGatekeeper::verifyUserSignature(const QString& username,
                                            const QByteArray& usernameSignature,
                                            const HifiSockAddr& senderSockAddr) {
diff --git a/domain-server/src/DomainGatekeeper.h b/domain-server/src/DomainGatekeeper.h
index 01cb6d1d3f..2cb9b4c8a9 100644
--- a/domain-server/src/DomainGatekeeper.h
+++ b/domain-server/src/DomainGatekeeper.h
@@ -39,8 +39,8 @@ public:
     void addPendingAssignedNode(const QUuid& nodeUUID, const QUuid& assignmentUUID,
                                 const QUuid& walletUUID, const QString& nodeVersion);
     QUuid assignmentUUIDForPendingAssignment(const QUuid& tempUUID);
-    
-    void removeICEPeer(const QUuid& peerUUID) { _icePeers.remove(peerUUID); }
+
+    void cleanupICEPeerForNode(const QUuid& nodeID);
 
     Node::LocalID findOrCreateLocalID(const QUuid& uuid);
 
@@ -100,6 +100,10 @@ private:
     std::unordered_map<QUuid, PendingAssignedNodeData> _pendingAssignedNodes;
     
     QHash<QUuid, SharedNetworkPeer> _icePeers;
+
+    using ConnectingNodeID = QUuid;
+    using ICEPeerID = QUuid;
+    QHash<ConnectingNodeID, ICEPeerID> _nodeToICEPeerIDs;
     
     QHash<QString, QUuid> _connectionTokenHash;
 
diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp
index baeac043e4..9010ff9e5f 100644
--- a/domain-server/src/DomainServer.cpp
+++ b/domain-server/src/DomainServer.cpp
@@ -1017,15 +1017,22 @@ void DomainServer::processListRequestPacket(QSharedPointer<ReceivedMessage> mess
     sendingNode->setPublicSocket(nodeRequestData.publicSockAddr);
     sendingNode->setLocalSocket(nodeRequestData.localSockAddr);
 
-    // update the NodeInterestSet in case there have been any changes
     DomainServerNodeData* nodeData = static_cast<DomainServerNodeData*>(sendingNode->getLinkedData());
 
+    if (!nodeData->hasCheckedIn()) {
+        nodeData->setHasCheckedIn(true);
+
+        // on first check in, make sure we've cleaned up any ICE peer for this node
+        _gatekeeper.cleanupICEPeerForNode(sendingNode->getUUID());
+    }
+
     // guard against patched agents asking to hear about other agents
     auto safeInterestSet = nodeRequestData.interestList.toSet();
     if (sendingNode->getType() == NodeType::Agent) {
         safeInterestSet.remove(NodeType::Agent);
     }
 
+    // update the NodeInterestSet in case there have been any changes
     nodeData->setNodeInterestSet(safeInterestSet);
 
     // update the connecting hostname in case it has changed
@@ -2945,7 +2952,7 @@ void DomainServer::nodeAdded(SharedNodePointer node) {
 
 void DomainServer::nodeKilled(SharedNodePointer node) {
     // if this peer connected via ICE then remove them from our ICE peers hash
-    _gatekeeper.removeICEPeer(node->getUUID());
+    _gatekeeper.cleanupICEPeerForNode(node->getUUID());
 
     DomainServerNodeData* nodeData = static_cast<DomainServerNodeData*>(node->getLinkedData());
 
diff --git a/domain-server/src/DomainServerNodeData.h b/domain-server/src/DomainServerNodeData.h
index 6b8e9a1718..f465cceb96 100644
--- a/domain-server/src/DomainServerNodeData.h
+++ b/domain-server/src/DomainServerNodeData.h
@@ -67,8 +67,11 @@ public:
     const QString& getPlaceName() { return _placeName; }
     void setPlaceName(const QString& placeName) { _placeName = placeName; }
 
-    bool wasAssigned() const { return _wasAssigned; };
+    bool wasAssigned() const { return _wasAssigned; }
     void setWasAssigned(bool wasAssigned) { _wasAssigned = wasAssigned; }
+
+    bool hasCheckedIn() const { return _hasCheckedIn; }
+    void setHasCheckedIn(bool hasCheckedIn) { _hasCheckedIn = hasCheckedIn; }
     
 private:
     QJsonObject overrideValuesIfNeeded(const QJsonObject& newStats);
@@ -94,6 +97,8 @@ private:
     QString _placeName;
 
     bool _wasAssigned { false };
+
+    bool _hasCheckedIn { false };
 };
 
 #endif // hifi_DomainServerNodeData_h