Merge pull request #1379 from ethereum/p2p

Bug fix. Add destination node to expected discovery ping (instead of host endpoint)

libp2p/Host.cpp

@@ -560,7 +560,7 @@ void Host::run(boost::system::error_code const&)
 		{
 			RecursiveGuard l(x_sessions);
 			for (auto p: m_peers)
-				if (p.second->shouldReconnect())
+				if (p.second->shouldReconnect() && !havePeerSession(p.second->id))
 					toConnect.push_back(p.second);
 		}
 		

libp2p/NodeTable.cpp

@@ -87,7 +87,8 @@ shared_ptr<NodeEntry> NodeTable::addNode(Node const& _node)
 	// ping address if nodeid is empty
 	if (!_node.id)
 	{
-		m_pubkDiscoverPings[m_node.endpoint.udp.address()] = std::chrono::steady_clock::now();
+		clog(NodeTableConnect) << "Sending public key discovery Ping to" << _node.endpoint.udp << "(Advertising:" << m_node.endpoint.udp << ")";
+		m_pubkDiscoverPings[_node.endpoint.udp.address()] = std::chrono::steady_clock::now();
 		PingNode p(_node.endpoint.udp, m_node.endpoint.udp.address().to_string(), m_node.endpoint.udp.port());
 		p.sign(m_secret);
 		m_socketPointer->send(p);
@@ -442,7 +443,10 @@ void NodeTable::onReceived(UDPSocketFace*, bi::udp::endpoint const& _from, bytes
 						n->pending = false;
 				}
 				else if (m_pubkDiscoverPings.count(_from.address()))
+				{
 					m_pubkDiscoverPings.erase(_from.address());
+					addNode(nodeid, _from, bi::tcp::endpoint(_from.address(), _from.port()));
+				}
 				else
 					return; // unsolicited pong; don't note node as active
 				

libp2p/NodeTable.h

@@ -316,13 +316,13 @@ struct PingNode: RLPXDatagram<PingNode>
 
 	static const uint8_t type = 1;
 
-	unsigned version = dev::p2p::c_protocolVersion;
+	unsigned version = 0;
 	std::string ipAddress;
 	unsigned port;
 	unsigned expiration;
 
-	void streamRLP(RLPStream& _s) const { _s.appendList(3); _s << ipAddress << port << expiration; }
-	void interpretRLP(bytesConstRef _bytes) { RLP r(_bytes); ipAddress = r[0].toString(); port = r[1].toInt<unsigned>(); expiration = r[2].toInt<unsigned>(); }
+	void streamRLP(RLPStream& _s) const { _s.appendList(4); _s << dev::p2p::c_protocolVersion << ipAddress << port << expiration; }
+	void interpretRLP(bytesConstRef _bytes) { RLP r(_bytes); version = r[0].toInt<unsigned>(); ipAddress = r[1].toString(); port = r[2].toInt<unsigned>(); expiration = r[3].toInt<unsigned>(); }
 };
 
 /**

libp2p/RLPxHandshake.cpp

@@ -252,7 +252,7 @@ void RLPXHandshake::transition(boost::system::error_code _ech)
 						}
 
 						clog(NetNote) << (m_originated ? "p2p.connect.egress" : "p2p.connect.ingress") << "hello frame: success. starting session.";
-						RLP rlp(frame.cropped(1));
+						RLP rlp(frame.cropped(1), RLP::ThrowOnFail | RLP::FailIfTooSmall);
 						m_host->startPeerSession(m_remote, rlp, m_io, m_socket->remoteEndpoint());
 					}
 				});

libp2p/UDP.h

@@ -77,7 +77,7 @@ template <class T>
 struct RLPXDatagram: public RLPXDatagramFace
 {
 	RLPXDatagram(bi::udp::endpoint const& _ep): RLPXDatagramFace(_ep) {}
-	static T fromBytesConstRef(bi::udp::endpoint const& _ep, bytesConstRef _bytes) { T t(_ep); t.interpretRLP(_bytes); return std::move(t); }
+	static T fromBytesConstRef(bi::udp::endpoint const& _ep, bytesConstRef _bytes) { try { T t(_ep); t.interpretRLP(_bytes); return std::move(t); } catch(...) { T t(_ep); return std::move(t); } }
 	uint8_t packetType() { return T::type; }
 };
 

test/net.cpp

@@ -145,6 +145,17 @@ public:
 	bool success = false;
 };
 
+BOOST_AUTO_TEST_CASE(badPingNodePacket)
+{
+	// test old versino of pingNode packet w/new
+	RLPStream s;
+	s.appendList(3); s << "1.1.1.1" << 30303 << std::chrono::duration_cast<std::chrono::seconds>((std::chrono::system_clock::now() + chrono::seconds(60)).time_since_epoch()).count();
+
+	PingNode p((bi::udp::endpoint()));
+	BOOST_REQUIRE_NO_THROW(p = PingNode::fromBytesConstRef(bi::udp::endpoint(), bytesConstRef(&s.out())));
+	BOOST_REQUIRE(p.version == 0);
+}
+
 BOOST_AUTO_TEST_CASE(test_neighbours_packet)
 {
 	KeyPair k = KeyPair::create();