Merge pull request #1587 from subtly/neighbours

Drop unsolicited neighbours packets.

libp2p/NodeTable.cpp

@@ -178,6 +178,7 @@ void NodeTable::discover(NodeId _node, unsigned _round, shared_ptr<set<shared_pt
 			tried.push_back(r);
 			FindNode p(r->endpoint.udp, _node);
 			p.sign(m_secret);
+			m_findNodeTimeout.push_back(make_pair(_node, chrono::steady_clock::now()));
 			m_socketPointer->send(p);
 		}
 	
@@ -457,6 +458,20 @@ void NodeTable::onReceived(UDPSocketFace*, bi::udp::endpoint const& _from, bytes
 				
 			case Neighbours::type:
 			{
+				bool expected = false;
+				m_findNodeTimeout.remove_if([&](NodeIdTimePoint const& t)
+				{
+					if (t.first == nodeid && chrono::steady_clock::now() - t.second < c_reqTimeout)
+						expected = true;
+					return t.first == nodeid;
+				});
+				
+				if (!expected)
+				{
+					clog(NetConnect) << "Dropping unsolicited Neighbours packet from " << _from.address();
+					break;
+				}
+				
 				Neighbours in = Neighbours::fromBytesConstRef(_from, rlpBytes);
 				for (auto n: in.nodes)
 					addNode(n.node, bi::udp::endpoint(bi::address::from_string(n.ipAddress), n.port), bi::tcp::endpoint(bi::address::from_string(n.ipAddress), n.port));

libp2p/NodeTable.h

@@ -131,8 +131,9 @@ class NodeTable: UDPSocketEvents, public std::enable_shared_from_this<NodeTable>
 {
 	friend std::ostream& operator<<(std::ostream& _out, NodeTable const& _nodeTable);
 	using NodeSocket = UDPSocket<NodeTable, 1280>;
-	using TimePoint = std::chrono::steady_clock::time_point;
-	using EvictionTimeout = std::pair<std::pair<NodeId, TimePoint>, NodeId>;	///< First NodeId may be evicted and replaced with second NodeId.
+	using TimePoint = std::chrono::steady_clock::time_point;	///< Steady time point.
+	using NodeIdTimePoint = std::pair<NodeId, TimePoint>;
+	using EvictionTimeout = std::pair<NodeIdTimePoint, NodeId>;	///< First NodeId (NodeIdTimePoint) may be evicted and replaced with second NodeId.
 
 public:
 	/// Constructor requiring host for I/O, credentials, and IP Address and port to listen on.
@@ -271,6 +272,9 @@ private:
 	Mutex x_pubkDiscoverPings;								///< LOCK x_nodes first if both x_nodes and x_pubkDiscoverPings locks are required.
 	std::map<bi::address, TimePoint> m_pubkDiscoverPings;		///< List of pending pings where node entry wasn't created due to unkown pubk.
 
+	Mutex x_findNodeTimeout;
+	std::list<NodeIdTimePoint> m_findNodeTimeout;				///< Timeouts for pending Ping and FindNode requests.
+	
 	ba::io_service& m_io;										///< Used by bucket refresh timer.
 	std::shared_ptr<NodeSocket> m_socket;						///< Shared pointer for our UDPSocket; ASIO requires shared_ptr.
 	NodeSocket* m_socketPointer;								///< Set to m_socket.get(). Socket is created in constructor and disconnected in destructor to ensure access to pointer is safe.

test/net.cpp

@@ -145,6 +145,39 @@ public:
 	bool success = false;
 };
 
+BOOST_AUTO_TEST_CASE(requestTimeout)
+{
+	using TimePoint = std::chrono::steady_clock::time_point;
+	using RequestTimeout = std::pair<NodeId, TimePoint>;
+	
+	std::chrono::milliseconds timeout(300);
+	std::list<RequestTimeout> timeouts;
+	
+	NodeId nodeA(sha3("a"));
+	NodeId nodeB(sha3("b"));
+	timeouts.push_back(make_pair(nodeA, chrono::steady_clock::now()));
+	this_thread::sleep_for(std::chrono::milliseconds(100));
+	timeouts.push_back(make_pair(nodeB, chrono::steady_clock::now()));
+	this_thread::sleep_for(std::chrono::milliseconds(210));
+	
+	bool nodeAtriggered = false;
+	bool nodeBtriggered = false;
+	timeouts.remove_if([&](RequestTimeout const& t)
+	{
+		auto now = chrono::steady_clock::now();
+		auto diff = now - t.second;
+		if (t.first == nodeA && diff < timeout)
+			nodeAtriggered = true;
+		if (t.first == nodeB && diff < timeout)
+			nodeBtriggered = true;
+		return (t.first == nodeA || t.first == nodeB);
+	});
+	
+	BOOST_REQUIRE(nodeAtriggered == false);
+	BOOST_REQUIRE(nodeBtriggered == true);
+	BOOST_REQUIRE(timeouts.size() == 0);
+}
+
 BOOST_AUTO_TEST_CASE(isIPAddressType)
 {
 	string wildcard = "0.0.0.0";