Browse Source

code review

cl-refactor
subtly 10 years ago
parent
commit
e9f4a7a439
  1. 13
      libp2p/NodeTable.cpp
  2. 26
      libp2p/NodeTable.h
  3. 11
      libp2p/UDP.h
  4. 12
      test/net.cpp

13
libp2p/NodeTable.cpp

@ -80,7 +80,7 @@ NodeTable::NodeEntry NodeTable::operator[](NodeId _id)
return *m_nodes[_id]; return *m_nodes[_id];
} }
void NodeTable::requestNeighbors(NodeEntry const& _node, NodeId _target) const void NodeTable::requestNeighbours(NodeEntry const& _node, NodeId _target) const
{ {
FindNode p(_node.endpoint.udp, _target); FindNode p(_node.endpoint.udp, _target);
p.sign(m_secret); p.sign(m_secret);
@ -312,7 +312,7 @@ NodeTable::NodeBucket& NodeTable::bucket(NodeEntry const* _n)
void NodeTable::onReceived(UDPSocketFace*, bi::udp::endpoint const& _from, bytesConstRef _packet) void NodeTable::onReceived(UDPSocketFace*, bi::udp::endpoint const& _from, bytesConstRef _packet)
{ {
// h256 + Signature + RLP (smallest possible packet is empty neighbors packet which is 3 bytes) // h256 + Signature + RLP (smallest possible packet is empty neighbours packet which is 3 bytes)
if (_packet.size() < h256::size + Signature::size + 3) if (_packet.size() < h256::size + Signature::size + 3)
{ {
clog(NodeTableMessageSummary) << "Invalid Message size from " << _from.address().to_string() << ":" << _from.port(); clog(NodeTableMessageSummary) << "Invalid Message size from " << _from.address().to_string() << ":" << _from.port();
@ -341,7 +341,8 @@ void NodeTable::onReceived(UDPSocketFace*, bi::udp::endpoint const& _from, bytes
noteNode(nodeid, _from); noteNode(nodeid, _from);
try { try {
switch (itemCount) { switch (itemCount)
{
case 1: case 1:
{ {
// clog(NodeTableMessageSummary) << "Received Pong from " << _from.address().to_string() << ":" << _from.port(); // clog(NodeTableMessageSummary) << "Received Pong from " << _from.address().to_string() << ":" << _from.port();
@ -355,8 +356,8 @@ void NodeTable::onReceived(UDPSocketFace*, bi::udp::endpoint const& _from, bytes
case 2: case 2:
if (rlp[0].isList()) if (rlp[0].isList())
{ {
Neighbors in = Neighbors::fromBytesConstRef(_from, rlpBytes); Neighbours in = Neighbours::fromBytesConstRef(_from, rlpBytes);
// clog(NodeTableMessageSummary) << "Received " << in.nodes.size() << " Neighbors from " << _from.address().to_string() << ":" << _from.port(); // clog(NodeTableMessageSummary) << "Received " << in.nodes.size() << " Neighbours from " << _from.address().to_string() << ":" << _from.port();
for (auto n: in.nodes) for (auto n: in.nodes)
noteNode(n.node, bi::udp::endpoint(bi::address::from_string(n.ipAddress), n.port)); noteNode(n.node, bi::udp::endpoint(bi::address::from_string(n.ipAddress), n.port));
} }
@ -369,7 +370,7 @@ void NodeTable::onReceived(UDPSocketFace*, bi::udp::endpoint const& _from, bytes
static unsigned const nlimit = (m_socketPtr->maxDatagramSize - 11) / 86; static unsigned const nlimit = (m_socketPtr->maxDatagramSize - 11) / 86;
for (unsigned offset = 0; offset < nearest.size(); offset += nlimit) for (unsigned offset = 0; offset < nearest.size(); offset += nlimit)
{ {
Neighbors out(_from, nearest, offset, nlimit); Neighbours out(_from, nearest, offset, nlimit);
out.sign(m_secret); out.sign(m_secret);
m_socketPtr->send(out); m_socketPtr->send(out);
} }

26
libp2p/NodeTable.h

@ -49,7 +49,7 @@ namespace p2p
* @todo Pong to include ip:port where ping was received * @todo Pong to include ip:port where ping was received
* @todo expiration and sha3(id) 'to' for messages which are replies (prevents replay) * @todo expiration and sha3(id) 'to' for messages which are replies (prevents replay)
* @todo std::shared_ptr<PingNode> m_cachedPingPacket; * @todo std::shared_ptr<PingNode> m_cachedPingPacket;
* @todo std::shared_ptr<FindNeighbors> m_cachedFindSelfPacket; * @todo std::shared_ptr<FindNeighbours> m_cachedFindSelfPacket;
* @todo store root node in table? * @todo store root node in table?
* *
* [Networking] * [Networking]
@ -66,7 +66,7 @@ namespace p2p
*/ */
class NodeTable: UDPSocketEvents, public std::enable_shared_from_this<NodeTable> class NodeTable: UDPSocketEvents, public std::enable_shared_from_this<NodeTable>
{ {
friend struct Neighbors; friend struct Neighbours;
using NodeSocket = UDPSocket<NodeTable, 1280>; using NodeSocket = UDPSocket<NodeTable, 1280>;
using TimePoint = std::chrono::steady_clock::time_point; 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 EvictionTimeout = std::pair<std::pair<NodeId,TimePoint>,NodeId>; ///< First NodeId may be evicted and replaced with second NodeId.
@ -180,7 +180,7 @@ private:
protected: protected:
#endif #endif
/// Sends FindNeighbor packet. See doFindNode. /// Sends FindNeighbor packet. See doFindNode.
void requestNeighbors(NodeEntry const& _node, NodeId _target) const; void requestNeighbours(NodeEntry const& _node, NodeId _target) const;
Node m_node; ///< This node. Node m_node; ///< This node.
Secret m_secret; ///< This nodes secret key. Secret m_secret; ///< This nodes secret key.
@ -269,7 +269,7 @@ struct Pong: RLPXDatagram<Pong>
/** /**
* FindNode Packet: Request k-nodes, closest to the target. * FindNode Packet: Request k-nodes, closest to the target.
* FindNode is cached and regenerated after expiration - t, where t is timeout. * FindNode is cached and regenerated after expiration - t, where t is timeout.
* FindNode implicitly results in finding neighbors of a given node. * FindNode implicitly results in finding neighbours of a given node.
* *
* RLP Encoded Items: 2 * RLP Encoded Items: 2
* Minimum Encoded Size: 21 bytes * Minimum Encoded Size: 21 bytes
@ -299,7 +299,7 @@ struct FindNode: RLPXDatagram<FindNode>
* *
* @todo nonce: Should be replaced with expiration. * @todo nonce: Should be replaced with expiration.
*/ */
struct Neighbors: RLPXDatagram<Neighbors> struct Neighbours: RLPXDatagram<Neighbours>
{ {
struct Node struct Node
{ {
@ -308,16 +308,12 @@ struct Neighbors: RLPXDatagram<Neighbors>
std::string ipAddress; std::string ipAddress;
unsigned port; unsigned port;
NodeId node; NodeId node;
void streamRLP(RLPStream& _s) const { void streamRLP(RLPStream& _s) const { _s.appendList(3); _s << ipAddress << port << node; }
_s.appendList(3); _s << ipAddress << port << node; void interpretRLP(RLP const& _r) { ipAddress = _r[0].toString(); port = _r[1].toInt<unsigned>(); node = h512(_r[2].toBytes()); }
}
void interpretRLP(RLP const& _r) {
ipAddress = _r[0].toString(); port = _r[1].toInt<unsigned>(); node = h512(_r[2].toBytes());
}
}; };
using RLPXDatagram<Neighbors>::RLPXDatagram; using RLPXDatagram<Neighbours>::RLPXDatagram;
Neighbors(bi::udp::endpoint _to, std::vector<std::shared_ptr<NodeTable::NodeEntry>> const& _nearest, unsigned _offset = 0, unsigned _limit = 0): RLPXDatagram<Neighbors>(_to) Neighbours(bi::udp::endpoint _to, std::vector<std::shared_ptr<NodeTable::NodeEntry>> const& _nearest, unsigned _offset = 0, unsigned _limit = 0): RLPXDatagram<Neighbours>(_to)
{ {
auto limit = _limit ? std::min(_nearest.size(), (size_t)(_offset + _limit)) : _nearest.size(); auto limit = _limit ? std::min(_nearest.size(), (size_t)(_offset + _limit)) : _nearest.size();
for (auto i = _offset; i < limit; i++) for (auto i = _offset; i < limit; i++)
@ -334,9 +330,7 @@ struct Neighbors: RLPXDatagram<Neighbors>
unsigned expiration = 1; unsigned expiration = 1;
void streamRLP(RLPStream& _s) const { _s.appendList(2); _s.appendList(nodes.size()); for (auto& n: nodes) n.streamRLP(_s); _s << expiration; } void streamRLP(RLPStream& _s) const { _s.appendList(2); _s.appendList(nodes.size()); for (auto& n: nodes) n.streamRLP(_s); _s << expiration; }
void interpretRLP(bytesConstRef _bytes) { void interpretRLP(bytesConstRef _bytes) { RLP r(_bytes); for (auto n: r[0]) nodes.push_back(Node(n)); expiration = r[1].toInt<unsigned>(); }
RLP r(_bytes); for (auto n: r[0]) nodes.push_back(Node(n)); expiration = r[1].toInt<unsigned>();
}
}; };
struct NodeTableWarn: public LogChannel { static const char* name() { return "!P!"; } static const int verbosity = 0; }; struct NodeTableWarn: public LogChannel { static const char* name() { return "!P!"; } static const int verbosity = 0; };

11
libp2p/UDP.h

@ -128,7 +128,6 @@ public:
void disconnect() { disconnectWithError(boost::asio::error::connection_reset); } void disconnect() { disconnectWithError(boost::asio::error::connection_reset); }
protected: protected:
void doRead(); void doRead();
void doWrite(); void doWrite();
@ -152,7 +151,7 @@ protected:
}; };
template <typename Handler, unsigned MaxDatagramSize> template <typename Handler, unsigned MaxDatagramSize>
void UDPSocket<Handler,MaxDatagramSize>::connect() void UDPSocket<Handler, MaxDatagramSize>::connect()
{ {
bool expect = false; bool expect = false;
if (!m_started.compare_exchange_strong(expect, true)) if (!m_started.compare_exchange_strong(expect, true))
@ -170,7 +169,7 @@ void UDPSocket<Handler,MaxDatagramSize>::connect()
} }
template <typename Handler, unsigned MaxDatagramSize> template <typename Handler, unsigned MaxDatagramSize>
bool UDPSocket<Handler,MaxDatagramSize>::send(UDPDatagram const& _datagram) bool UDPSocket<Handler, MaxDatagramSize>::send(UDPDatagram const& _datagram)
{ {
if (m_closed) if (m_closed)
return false; return false;
@ -184,7 +183,7 @@ bool UDPSocket<Handler,MaxDatagramSize>::send(UDPDatagram const& _datagram)
} }
template <typename Handler, unsigned MaxDatagramSize> template <typename Handler, unsigned MaxDatagramSize>
void UDPSocket<Handler,MaxDatagramSize>::doRead() void UDPSocket<Handler, MaxDatagramSize>::doRead()
{ {
if (m_closed) if (m_closed)
return; return;
@ -202,7 +201,7 @@ void UDPSocket<Handler,MaxDatagramSize>::doRead()
} }
template <typename Handler, unsigned MaxDatagramSize> template <typename Handler, unsigned MaxDatagramSize>
void UDPSocket<Handler,MaxDatagramSize>::doWrite() void UDPSocket<Handler, MaxDatagramSize>::doWrite()
{ {
if (m_closed) if (m_closed)
return; return;
@ -225,7 +224,7 @@ void UDPSocket<Handler,MaxDatagramSize>::doWrite()
} }
template <typename Handler, unsigned MaxDatagramSize> template <typename Handler, unsigned MaxDatagramSize>
void UDPSocket<Handler,MaxDatagramSize>::disconnectWithError(boost::system::error_code _ec) void UDPSocket<Handler, MaxDatagramSize>::disconnectWithError(boost::system::error_code _ec)
{ {
// If !started and already stopped, shutdown has already occured. (EOF or Operation canceled) // If !started and already stopped, shutdown has already occured. (EOF or Operation canceled)
if (!m_started && m_closed && !m_socket.is_open() /* todo: veirfy this logic*/) if (!m_started && m_closed && !m_socket.is_open() /* todo: veirfy this logic*/)

12
test/net.cpp

@ -134,16 +134,16 @@ public:
bool success = false; bool success = false;
}; };
BOOST_AUTO_TEST_CASE(test_neighbors_packet) BOOST_AUTO_TEST_CASE(test_neighbours_packet)
{ {
KeyPair k = KeyPair::create(); KeyPair k = KeyPair::create();
std::vector<std::pair<KeyPair,unsigned>> testNodes(TestNodeTable::createTestNodes(16)); std::vector<std::pair<KeyPair,unsigned>> testNodes(TestNodeTable::createTestNodes(16));
bi::udp::endpoint to(boost::asio::ip::address::from_string("127.0.0.1"), 30000); bi::udp::endpoint to(boost::asio::ip::address::from_string("127.0.0.1"), 30000);
Neighbors out(to); Neighbours out(to);
for (auto n: testNodes) for (auto n: testNodes)
{ {
Neighbors::Node node; Neighbours::Node node;
node.ipAddress = boost::asio::ip::address::from_string("127.0.0.1").to_string(); node.ipAddress = boost::asio::ip::address::from_string("127.0.0.1").to_string();
node.port = n.second; node.port = n.second;
node.node = n.first.pub(); node.node = n.first.pub();
@ -153,7 +153,7 @@ BOOST_AUTO_TEST_CASE(test_neighbors_packet)
bytesConstRef packet(out.data.data(), out.data.size()); bytesConstRef packet(out.data.data(), out.data.size());
bytesConstRef rlpBytes(packet.cropped(97, packet.size() - 97)); bytesConstRef rlpBytes(packet.cropped(97, packet.size() - 97));
Neighbors in = Neighbors::fromBytesConstRef(to, rlpBytes); Neighbours in = Neighbours::fromBytesConstRef(to, rlpBytes);
int count = 0; int count = 0;
for (auto n: in.nodes) for (auto n: in.nodes)
{ {
@ -164,10 +164,10 @@ BOOST_AUTO_TEST_CASE(test_neighbors_packet)
} }
} }
BOOST_AUTO_TEST_CASE(test_findnode_neighbors) BOOST_AUTO_TEST_CASE(test_findnode_neighbours)
{ {
// Executing findNode should result in a list which is serialized // Executing findNode should result in a list which is serialized
// into Neighbors packet. Neighbors packet should then be deserialized // into Neighbours packet. Neighbours packet should then be deserialized
// into the same list of nearest nodes. // into the same list of nearest nodes.
} }

Loading…
Cancel
Save