Skip to content
This repository was archived by the owner on Oct 28, 2021. It is now read-only.

Commit 5373edf

Browse files
committed
m_sentPings maps pinged udp::endpoint to NodeValidation
1 parent 258c891 commit 5373edf

File tree

3 files changed

+43
-32
lines changed

3 files changed

+43
-32
lines changed

libp2p/NodeTable.cpp

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ void NodeTable::ping(Node const& _node, shared_ptr<NodeEntry> _replacementNodeEn
322322
return;
323323

324324
// Don't sent Ping if one is already sent
325-
if (contains(m_sentPings, _node.id))
325+
if (m_sentPings.find(_node.endpoint) != m_sentPings.end())
326326
{
327327
LOG(m_logger) << "Ignoring request to ping " << _node << ", because it's already pinged";
328328
return;
@@ -334,8 +334,9 @@ void NodeTable::ping(Node const& _node, shared_ptr<NodeEntry> _replacementNodeEn
334334
LOG(m_logger) << p.typeName() << " to " << _node;
335335
m_socket->send(p);
336336

337-
m_sentPings[_node.id] = {
338-
_node.endpoint, chrono::steady_clock::now(), pingHash, move(_replacementNodeEntry)};
337+
NodeValidation const validation(_node.id, _node.endpoint.tcpPort(), chrono::steady_clock::now(),
338+
pingHash, _replacementNodeEntry);
339+
m_sentPings.insert({_node.endpoint, validation});
339340
}
340341

341342
void NodeTable::schedulePing(Node const& _node)
@@ -470,18 +471,16 @@ void NodeTable::onPacketReceived(
470471
{
471472
case Pong::type:
472473
{
473-
auto const& pong = dynamic_cast<Pong const&>(*packet);
474-
auto const& sourceId = pong.sourceid;
475-
476474
// validate pong
477-
auto const sentPing = m_sentPings.find(sourceId);
475+
auto const sentPing = m_sentPings.find(_from);
478476
if (sentPing == m_sentPings.end())
479477
{
480478
LOG(m_logger) << "Unexpected PONG from " << _from.address().to_string() << ":"
481479
<< _from.port();
482480
return;
483481
}
484482

483+
auto const& pong = dynamic_cast<Pong const&>(*packet);
485484
if (pong.echo != sentPing->second.pingHash)
486485
{
487486
LOG(m_logger) << "Invalid PONG from " << _from.address().to_string() << ":"
@@ -492,10 +491,12 @@ void NodeTable::onPacketReceived(
492491
// create or update nodeEntry with new Pong received time
493492
DEV_GUARDED(x_nodes)
494493
{
494+
auto const& sourceId = pong.sourceid;
495495
auto it = m_allNodes.find(sourceId);
496496
if (it == m_allNodes.end())
497497
sourceNodeEntry = make_shared<NodeEntry>(m_hostNodeID, sourceId,
498-
sentPing->second.endpoint, RLPXDatagramFace::secondsSinceEpoch(), 0);
498+
NodeIPEndpoint{_from.address(), _from.port(), sentPing->second.tcpPort},
499+
RLPXDatagramFace::secondsSinceEpoch(), 0);
499500
else
500501
{
501502
sourceNodeEntry = it->second;
@@ -504,7 +505,7 @@ void NodeTable::onPacketReceived(
504505
}
505506
}
506507

507-
m_sentPings.erase(sentPing);
508+
m_sentPings.erase(_from);
508509

509510
// update our endpoint address and UDP port
510511
DEV_GUARDED(x_nodes)
@@ -682,7 +683,7 @@ void NodeTable::doHandleTimeouts()
682683
{
683684
if (chrono::steady_clock::now() > it->second.pingSendTime + m_requestTimeToLive)
684685
{
685-
if (auto node = nodeEntry(it->first))
686+
if (auto node = nodeEntry(it->second.nodeID))
686687
{
687688
dropNode(move(node));
688689

libp2p/NodeTable.h

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,15 +177,25 @@ class NodeTable : UDPSocketEvents
177177
// protected only for derived classes in tests
178178
protected:
179179
/**
180-
* NodeValidation is used to record Pinged node's endpoint, the timepoint of sent PING,
180+
* NodeValidation is used to record Pinged node's ID, the timepoint of sent PING,
181181
* time of sending and the new node ID to replace unresponsive node.
182182
*/
183183
struct NodeValidation
184184
{
185-
NodeIPEndpoint endpoint;
185+
NodeID nodeID;
186+
uint16_t tcpPort = 0;
186187
TimePoint pingSendTime;
187188
h256 pingHash;
188189
std::shared_ptr<NodeEntry> replacementNodeEntry;
190+
191+
NodeValidation(NodeID const& _nodeID, uint16_t _tcpPort, TimePoint _pingSendTime,
192+
h256 const& _pingHash, std::shared_ptr<NodeEntry> _replacementNodeEntry)
193+
: nodeID(_nodeID),
194+
tcpPort(_tcpPort),
195+
pingSendTime(_pingSendTime),
196+
pingHash(_pingHash),
197+
replacementNodeEntry(std::move(_replacementNodeEntry))
198+
{}
189199
};
190200

191201
/// Constants for Kademlia, derived from address space.
@@ -308,7 +318,7 @@ class NodeTable : UDPSocketEvents
308318
std::shared_ptr<NodeSocket> m_socket; ///< Shared pointer for our UDPSocket; ASIO requires shared_ptr.
309319

310320
// The info about PING packets we've sent to other nodes and haven't received PONG yet
311-
std::unordered_map<NodeID, NodeValidation> m_sentPings;
321+
std::map<bi::udp::endpoint, NodeValidation> m_sentPings;
312322

313323
// Expiration time of sent discovery packets.
314324
std::chrono::seconds m_requestTimeToLive;

test/unittests/libp2p/net.cpp

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,11 @@ struct TestNodeTable: public NodeTable
185185
return m_buckets[_bucket].nodes.back().lock();
186186
}
187187

188-
boost::optional<NodeValidation> nodeValidation(NodeID const& _id)
188+
boost::optional<NodeValidation> nodeValidation(bi::udp::endpoint const& _endpoint)
189189
{
190190
promise<boost::optional<NodeValidation>> promise;
191-
m_io.post([this, &promise, _id] {
192-
auto validation = m_sentPings.find(_id);
191+
m_io.post([this, &promise, _endpoint] {
192+
auto validation = m_sentPings.find(_endpoint);
193193
if (validation != m_sentPings.end())
194194
promise.set_value(validation->second);
195195
else
@@ -544,7 +544,7 @@ BOOST_AUTO_TEST_CASE(noteActiveNodeEvictsTheNodeWhenBucketIsFull)
544544
// least recently seen node not removed yet
545545
BOOST_CHECK_EQUAL(nodeTable->bucketFirstNode(bucketIndex), leastRecentlySeenNode);
546546
// but added to evictions
547-
auto evicted = nodeTable->nodeValidation(leastRecentlySeenNode->id);
547+
auto evicted = nodeTable->nodeValidation(leastRecentlySeenNode->endpoint);
548548
BOOST_REQUIRE(evicted.is_initialized());
549549
BOOST_REQUIRE(evicted->replacementNodeEntry);
550550
BOOST_CHECK_EQUAL(evicted->replacementNodeEntry->id, newNodeId);
@@ -632,7 +632,7 @@ BOOST_AUTO_TEST_CASE(invalidPong)
632632
nodeTable->packetsReceived.pop();
633633

634634
// pending node validation should still be not deleted
635-
BOOST_REQUIRE(nodeTable->nodeValidation(nodePubKey));
635+
BOOST_REQUIRE(nodeTable->nodeValidation(nodeEndpoint));
636636
// node is not in the node table
637637
BOOST_REQUIRE(!nodeTable->nodeExists(nodePubKey));
638638
}
@@ -684,7 +684,7 @@ BOOST_AUTO_TEST_CASE(pongWithChangedNodeID)
684684
nodeTable->setRequestTimeToLive(std::chrono::seconds(1));
685685

686686
// socket receiving PING
687-
TestUDPSocketHost nodeSocketHost{getRandomPortNumber()};
687+
TestUDPSocketHost nodeSocketHost{randomPortNumber()};
688688
nodeSocketHost.start();
689689
uint16_t nodePort = nodeSocketHost.port;
690690

@@ -719,7 +719,7 @@ BOOST_AUTO_TEST_CASE(pongWithChangedNodeID)
719719
this_thread::sleep_for(std::chrono::seconds(6));
720720

721721
BOOST_CHECK(!nodeTable->nodeExists(nodePubKey));
722-
auto sentPing = nodeTable->nodeValidation(nodePubKey);
722+
auto sentPing = nodeTable->nodeValidation(nodeEndpoint);
723723
BOOST_CHECK(!sentPing.is_initialized());
724724
}
725725

@@ -746,7 +746,7 @@ BOOST_AUTO_TEST_CASE(pingTimeout)
746746
this_thread::sleep_for(chrono::seconds(6));
747747

748748
BOOST_CHECK(!nodeTable->nodeExists(nodePubKey));
749-
auto sentPing = nodeTable->nodeValidation(nodePubKey);
749+
auto sentPing = nodeTable->nodeValidation(nodeEndpoint);
750750
BOOST_CHECK(!sentPing.is_initialized());
751751

752752
// handle received PING
@@ -765,7 +765,7 @@ BOOST_AUTO_TEST_CASE(pingTimeout)
765765
nodeTable->packetsReceived.pop();
766766

767767
BOOST_CHECK(!nodeTable->nodeExists(nodePubKey));
768-
sentPing = nodeTable->nodeValidation(nodePubKey);
768+
sentPing = nodeTable->nodeValidation(nodeEndpoint);
769769
BOOST_CHECK(!sentPing.is_initialized());
770770
}
771771

@@ -897,7 +897,7 @@ BOOST_AUTO_TEST_CASE(evictionWithOldNodeAnswering)
897897
// wait for eviction
898898
evictEvents.pop(chrono::seconds(5));
899899

900-
auto evicted = nodeTable->nodeValidation(nodeId);
900+
auto evicted = nodeTable->nodeValidation(nodeEndpoint);
901901
BOOST_REQUIRE(evicted.is_initialized());
902902

903903
// handle received PING
@@ -919,7 +919,7 @@ BOOST_AUTO_TEST_CASE(evictionWithOldNodeAnswering)
919919
BOOST_REQUIRE(nodeTable->nodeExists(nodeId));
920920
auto addedNode = nodeTable->nodeEntry(nodeId);
921921
BOOST_CHECK(addedNode->lastPongReceivedTime);
922-
auto sentPing = nodeTable->nodeValidation(nodeId);
922+
auto sentPing = nodeTable->nodeValidation(nodeEndpoint);
923923
BOOST_CHECK(!sentPing.is_initialized());
924924
// check that old node is most recently seen in the bucket
925925
BOOST_CHECK(nodeTable->bucketLastNode(bucketIndex)->id == nodeId);
@@ -938,7 +938,7 @@ BOOST_AUTO_TEST_CASE(evictionWithOldNodeDropped)
938938

939939
nodeTableHost.start();
940940

941-
auto oldNodeId = nodeTable->bucketFirstNode(bucketIndex)->id;
941+
auto oldNode = nodeTable->bucketFirstNode(bucketIndex);
942942

943943
// generate new address for the same bucket
944944
NodeID newNodeId;
@@ -958,8 +958,8 @@ BOOST_AUTO_TEST_CASE(evictionWithOldNodeDropped)
958958
this_thread::sleep_for(chrono::seconds(6));
959959

960960
// check that old node is evicted
961-
BOOST_CHECK(!nodeTable->nodeExists(oldNodeId));
962-
BOOST_CHECK(!nodeTable->nodeValidation(oldNodeId).is_initialized());
961+
BOOST_CHECK(!nodeTable->nodeExists(oldNode->id));
962+
BOOST_CHECK(!nodeTable->nodeValidation(oldNode->endpoint).is_initialized());
963963
// check that replacement node is active
964964
BOOST_CHECK(nodeTable->nodeExists(newNodeId));
965965
auto newNode = nodeTable->nodeEntry(newNodeId);
@@ -1019,12 +1019,12 @@ BOOST_AUTO_TEST_CASE(addSelf)
10191019
auto nodePubKey = KeyPair::create().pub();
10201020
Node node(nodePubKey, nodeEndpoint);
10211021
nodeTable->addNode(node);
1022-
BOOST_CHECK(nodeTable->nodeValidation(nodePubKey));
1022+
BOOST_CHECK(nodeTable->nodeValidation(nodeEndpoint));
10231023

10241024
// Create self node and verify it isn't pinged
1025-
Node self(nodeTableHost.m_alias.pub(), nodeEndpoint);
1025+
Node self(nodeTableHost.m_alias.pub(), nodeTable->m_hostNodeEndpoint);
10261026
nodeTable->addNode(self);
1027-
BOOST_CHECK(!nodeTable->nodeValidation(nodeTableHost.m_alias.pub()));
1027+
BOOST_CHECK(!nodeTable->nodeValidation(nodeTable->m_hostNodeEndpoint));
10281028
}
10291029

10301030
BOOST_AUTO_TEST_CASE(findNodeIsSentAfterPong)
@@ -1156,15 +1156,15 @@ BOOST_AUTO_TEST_CASE(addNodePingsNodeOnlyOnce)
11561156
auto nodePubKey = KeyPair::create().pub();
11571157
nodeTable->addNode(Node{nodePubKey, nodeEndpoint});
11581158

1159-
auto sentPing = nodeTable->nodeValidation(nodePubKey);
1159+
auto sentPing = nodeTable->nodeValidation(nodeEndpoint);
11601160
BOOST_REQUIRE(sentPing.is_initialized());
11611161

11621162
this_thread::sleep_for(chrono::milliseconds(2000));
11631163

11641164
// add it for the second time
11651165
nodeTable->addNode(Node{nodePubKey, nodeEndpoint});
11661166

1167-
auto sentPing2 = nodeTable->nodeValidation(nodePubKey);
1167+
auto sentPing2 = nodeTable->nodeValidation(nodeEndpoint);
11681168
BOOST_REQUIRE(sentPing2.is_initialized());
11691169

11701170
// check that Ping was sent only once, so Ping hash didn't change

0 commit comments

Comments
 (0)