From db14df5d0d69252ff59934fad7fd5861458b9fd4 Mon Sep 17 00:00:00 2001 From: Alden Torres Date: Sun, 18 Sep 2016 20:08:15 -0400 Subject: [PATCH] dht code related refactor and cleanup (#1107) dht code related refactor and cleanup --- include/libtorrent/kademlia/dht_storage.hpp | 3 +- include/libtorrent/kademlia/msg.hpp | 6 -- include/libtorrent/kademlia/node.hpp | 2 +- include/libtorrent/kademlia/node_id.hpp | 7 +- include/libtorrent/sha1_hash.hpp | 2 - src/kademlia/dht_storage.cpp | 88 +++++++++------------ src/kademlia/node.cpp | 12 +-- src/kademlia/node_id.cpp | 4 +- src/kademlia/routing_table.cpp | 2 +- test/test_dht.cpp | 28 ++++--- 10 files changed, 65 insertions(+), 89 deletions(-) diff --git a/include/libtorrent/kademlia/dht_storage.hpp b/include/libtorrent/kademlia/dht_storage.hpp index 277e398ec..e817846cf 100644 --- a/include/libtorrent/kademlia/dht_storage.hpp +++ b/include/libtorrent/kademlia/dht_storage.hpp @@ -39,7 +39,6 @@ POSSIBILITY OF SUCH DAMAGE. #include #include -#include #include #include #include @@ -109,7 +108,7 @@ namespace dht // If the torrent tracked contains a name, such a name // must be stored as a string in peers["n"] // - // If the scrape parameter is true, you should fill these keys:: + // If the scrape parameter is true, you should fill these keys: // // peers["BFpe"] - with the standard bit representation of a // 256 bloom filter containing the downloaders diff --git a/include/libtorrent/kademlia/msg.hpp b/include/libtorrent/kademlia/msg.hpp index fc63ee329..f1102b04a 100644 --- a/include/libtorrent/kademlia/msg.hpp +++ b/include/libtorrent/kademlia/msg.hpp @@ -33,9 +33,7 @@ POSSIBILITY OF SUCH DAMAGE. #ifndef TORRENT_KADEMLIA_MSG_HPP #define TORRENT_KADEMLIA_MSG_HPP -#include #include "libtorrent/socket.hpp" -#include "libtorrent/kademlia/node_id.hpp" namespace libtorrent { @@ -44,10 +42,6 @@ class entry; namespace dht { -typedef std::vector packet_t; -typedef std::vector nodes_t; -typedef std::vector peers_t; - struct msg { msg(bdecode_node const& m, udp::endpoint const& ep): message(m), addr(ep) {} diff --git a/include/libtorrent/kademlia/node.hpp b/include/libtorrent/kademlia/node.hpp index 6b08de1a1..4a33fddae 100644 --- a/include/libtorrent/kademlia/node.hpp +++ b/include/libtorrent/kademlia/node.hpp @@ -72,7 +72,7 @@ namespace libtorrent { namespace dht struct traversal_algorithm; struct dht_observer; -TORRENT_EXTRA_EXPORT void write_nodes_entry(entry& r, nodes_t const& nodes); +TORRENT_EXTRA_EXPORT entry write_nodes_entry(std::vector const& nodes); struct null_type {}; diff --git a/include/libtorrent/kademlia/node_id.hpp b/include/libtorrent/kademlia/node_id.hpp index d5ac43af9..580be7b51 100644 --- a/include/libtorrent/kademlia/node_id.hpp +++ b/include/libtorrent/kademlia/node_id.hpp @@ -32,19 +32,16 @@ POSSIBILITY OF SUCH DAMAGE. #ifndef NODE_ID_HPP #define NODE_ID_HPP -#include #include #include #include -#include +#include #include namespace libtorrent { namespace dht { -struct node_entry; - using node_id = libtorrent::sha1_hash; // returns the distance between the two nodes @@ -70,7 +67,7 @@ TORRENT_EXTRA_EXPORT bool verify_secret_id(node_id const& nid); TORRENT_EXTRA_EXPORT node_id generate_id_impl(address const& ip_, std::uint32_t r); TORRENT_EXTRA_EXPORT bool verify_id(node_id const& nid, address const& source_ip); -TORRENT_EXTRA_EXPORT bool matching_prefix(node_entry const& n, int mask, int prefix, int offset); +TORRENT_EXTRA_EXPORT bool matching_prefix(node_id const& nid, int mask, int prefix, int offset); TORRENT_EXTRA_EXPORT node_id generate_prefix_mask(int bits); } } // namespace libtorrent::dht diff --git a/include/libtorrent/sha1_hash.hpp b/include/libtorrent/sha1_hash.hpp index f5f5cff3c..c5ee9fe07 100644 --- a/include/libtorrent/sha1_hash.hpp +++ b/include/libtorrent/sha1_hash.hpp @@ -277,8 +277,6 @@ namespace libtorrent // peer IDs, node IDs etc. using sha1_hash = digest32<160>; - typedef sha1_hash peer_id; - #if TORRENT_USE_IOSTREAM // print a sha1_hash object to an ostream as 40 hexadecimal digits diff --git a/src/kademlia/dht_storage.cpp b/src/kademlia/dht_storage.cpp index f388ae8c3..2c4f46867 100644 --- a/src/kademlia/dht_storage.cpp +++ b/src/kademlia/dht_storage.cpp @@ -43,23 +43,14 @@ POSSIBILITY OF SUCH DAMAGE. #include #include #include -#include -#include #include -#include #include -#include #include -#include -#include - namespace libtorrent { namespace dht { namespace { - using detail::write_endpoint; - // this is the entry for every peer // the timestamp is there to make it possible // to remove stale peers @@ -112,16 +103,16 @@ namespace std::string salt; }; - void touch_item(dht_immutable_item* f, address const& addr) + void touch_item(dht_immutable_item& f, address const& addr) { - f->last_seen = aux::time_now(); + f.last_seen = aux::time_now(); // maybe increase num_announcers if we haven't seen this IP before sha1_hash const iphash = hash_address(addr); - if (!f->ips.find(iphash)) + if (!f.ips.find(iphash)) { - f->ips.set(iphash); - ++f->num_announcers; + f.ips.set(iphash); + ++f.num_announcers; } } @@ -169,10 +160,6 @@ namespace class dht_default_storage final : public dht_storage_interface, boost::noncopyable { - using table_t = std::map; - using dht_immutable_table_t = std::map; - using dht_mutable_table_t = std::map; - public: explicit dht_default_storage(dht_settings const& settings) @@ -198,11 +185,11 @@ namespace m_node_ids = ids; } - bool get_peers(sha1_hash const& info_hash, udp protocol + bool get_peers(sha1_hash const& info_hash, udp const protocol , bool const noseed, bool const scrape , entry& peers) const override { - table_t::const_iterator i = m_map.lower_bound(info_hash); + auto const i = m_map.lower_bound(info_hash); if (i == m_map.end()) return false; if (i->first != info_hash) return false; @@ -234,7 +221,7 @@ namespace if (!v.peers.empty() && protocol == udp::v6()) max /= 4; // we're picking "to_pick" from a list of "num" at random. - int const to_pick = (std::min)(int(v.peers.size()), max); + int const to_pick = std::min(int(v.peers.size()), max); std::set::const_iterator iter = v.peers.begin(); entry::list_type& pe = peers["values"].list(); @@ -258,13 +245,13 @@ namespace else { // maybe replace an item we've already picked - if (random(t-1) >= to_pick) continue; + if (random(t - 1) >= to_pick) continue; str = &pe[random(to_pick - 1)].string(); } str->resize(18); std::string::iterator out = str->begin(); - write_endpoint(iter->addr, out); + detail::write_endpoint(iter->addr, out); str->resize(out - str->begin()); ++m; @@ -277,7 +264,7 @@ namespace , tcp::endpoint const& endp , string_view name, bool const seed) override { - table_t::iterator ti = m_map.find(info_hash); + auto const ti = m_map.find(info_hash); torrent_entry* v; if (ti == m_map.end()) { @@ -288,8 +275,8 @@ namespace // we need to remove some. Remove the ones with the // fewest peers int num_peers = int(m_map.begin()->second.peers.size()); - table_t::iterator candidate = m_map.begin(); - for (table_t::iterator i = m_map.begin() + auto candidate = m_map.begin(); + for (auto i = m_map.begin() , end(m_map.end()); i != end; ++i) { if (int(i->second.peers.size()) > num_peers) continue; @@ -320,7 +307,7 @@ namespace peer.addr = endp; peer.added = aux::time_now(); peer.seed = seed; - std::set::iterator i = v->peers.find(peer); + auto i = v->peers.find(peer); if (i != v->peers.end()) { v->peers.erase(i++); @@ -343,7 +330,7 @@ namespace bool get_immutable_item(sha1_hash const& target , entry& item) const override { - dht_immutable_table_t::const_iterator i = m_immutable_table.find(target); + auto const i = m_immutable_table.find(target); if (i == m_immutable_table.end()) return false; item["v"] = bdecode(i->second.value.get() @@ -356,13 +343,13 @@ namespace , address const& addr) override { TORRENT_ASSERT(!m_node_ids.empty()); - dht_immutable_table_t::iterator i = m_immutable_table.find(target); + auto i = m_immutable_table.find(target); if (i == m_immutable_table.end()) { // make sure we don't add too many items if (int(m_immutable_table.size()) >= m_settings.max_dht_items) { - auto j = pick_least_important_item(m_node_ids + auto const j = pick_least_important_item(m_node_ids , m_immutable_table); TORRENT_ASSERT(j != m_immutable_table.end()); @@ -372,7 +359,7 @@ namespace dht_immutable_item to_add; to_add.value.reset(new char[buf.size()]); to_add.size = int(buf.size()); - memcpy(to_add.value.get(), buf.data(), buf.size()); + std::memcpy(to_add.value.get(), buf.data(), buf.size()); std::tie(i, std::ignore) = m_immutable_table.insert( std::make_pair(target, std::move(to_add))); @@ -381,13 +368,13 @@ namespace // std::fprintf(stderr, "added immutable item (%d)\n", int(m_immutable_table.size())); - touch_item(&i->second, addr); + touch_item(i->second, addr); } bool get_mutable_item_seq(sha1_hash const& target , sequence_number& seq) const override { - dht_mutable_table_t::const_iterator i = m_mutable_table.find(target); + auto const i = m_mutable_table.find(target); if (i == m_mutable_table.end()) return false; seq = i->second.seq; @@ -395,10 +382,10 @@ namespace } bool get_mutable_item(sha1_hash const& target - , sequence_number seq, bool force_fill + , sequence_number const seq, bool const force_fill , entry& item) const override { - dht_mutable_table_t::const_iterator i = m_mutable_table.find(target); + auto const i = m_mutable_table.find(target); if (i == m_mutable_table.end()) return false; dht_mutable_item const& f = i->second; @@ -415,20 +402,20 @@ namespace void put_mutable_item(sha1_hash const& target , span buf , signature const& sig - , sequence_number seq + , sequence_number const seq , public_key const& pk , span salt , address const& addr) override { TORRENT_ASSERT(!m_node_ids.empty()); - dht_mutable_table_t::iterator i = m_mutable_table.find(target); + auto i = m_mutable_table.find(target); if (i == m_mutable_table.end()) { // this is the case where we don't have an item in this slot // make sure we don't add too many items if (int(m_mutable_table.size()) >= m_settings.max_dht_items) { - auto j = pick_least_important_item(m_node_ids + auto const j = pick_least_important_item(m_node_ids , m_mutable_table); TORRENT_ASSERT(j != m_mutable_table.end()); @@ -442,7 +429,7 @@ namespace to_add.salt.assign(salt.data(), salt.size()); to_add.sig = sig; to_add.key = pk; - memcpy(to_add.value.get(), buf.data(), buf.size()); + std::memcpy(to_add.value.get(), buf.data(), buf.size()); std::tie(i, std::ignore) = m_mutable_table.insert( std::make_pair(target, std::move(to_add))); @@ -462,19 +449,19 @@ namespace } item.seq = seq; item.sig = sig; - memcpy(item.value.get(), buf.data(), buf.size()); + std::memcpy(item.value.get(), buf.data(), buf.size()); } } - touch_item(&i->second, addr); + touch_item(i->second, addr); } void tick() override { - time_point now(aux::time_now()); + time_point const now(aux::time_now()); // look through all peers and see if any have timed out - for (table_t::iterator i = m_map.begin(), end(m_map.end()); i != end;) + for (auto i = m_map.begin(), end(m_map.end()); i != end;) { torrent_entry& t = i->second; purge_peers(t.peers); @@ -496,8 +483,7 @@ namespace // item lifetime must >= 120 minutes. if (lifetime < minutes(120)) lifetime = minutes(120); - for (dht_immutable_table_t::iterator i = m_immutable_table.begin(); - i != m_immutable_table.end();) + for (auto i = m_immutable_table.begin(); i != m_immutable_table.end();) { if (i->second.last_seen + lifetime > now) { @@ -508,8 +494,7 @@ namespace m_counters.immutable_data -= 1; } - for (dht_mutable_table_t::iterator i = m_mutable_table.begin(); - i != m_mutable_table.end();) + for (auto i = m_mutable_table.begin(); i != m_mutable_table.end();) { if (i->second.last_seen + lifetime > now) { @@ -531,14 +516,13 @@ namespace dht_storage_counters m_counters; std::vector m_node_ids; - table_t m_map; - dht_immutable_table_t m_immutable_table; - dht_mutable_table_t m_mutable_table; + std::map m_map; + std::map m_immutable_table; + std::map m_mutable_table; void purge_peers(std::set& peers) { - for (std::set::iterator i = peers.begin() - , end(peers.end()); i != end;) + for (auto i = peers.begin(), end(peers.end()); i != end;) { // the peer has timed out if (i->added + minutes(int(announce_interval * 3 / 2)) < aux::time_now()) diff --git a/src/kademlia/node.cpp b/src/kademlia/node.cpp index d738384a6..f78e1ca7d 100644 --- a/src/kademlia/node.cpp +++ b/src/kademlia/node.cpp @@ -759,14 +759,16 @@ void node::lookup_peers(sha1_hash const& info_hash, entry& reply m_storage.get_peers(info_hash, protocol(), noseed, scrape, reply); } -void write_nodes_entry(entry& r, nodes_t const& nodes) +entry write_nodes_entry(std::vector const& nodes) { + entry r; std::back_insert_iterator out(r.string()); for (auto const& n : nodes) { std::copy(n.id.begin(), n.id.end(), out); write_endpoint(udp::endpoint(n.addr(), n.port()), out); } + return r; } // build response @@ -1160,9 +1162,9 @@ void node::write_nodes_entries(sha1_hash const& info_hash // entry based on the protocol the request came in with if (want.type() != bdecode_node::list_t) { - nodes_t n; + std::vector n; m_table.find_node(info_hash, n, 0); - write_nodes_entry(r[protocol_nodes_key()], n); + r[protocol_nodes_key()] = write_nodes_entry(n); return; } @@ -1178,9 +1180,9 @@ void node::write_nodes_entries(sha1_hash const& info_hash continue; auto wanted_node = m_nodes.find(wanted.string_value().to_string()); if (wanted_node == m_nodes.end()) continue; - nodes_t n; + std::vector n; wanted_node->second->m_table.find_node(info_hash, n, 0); - write_nodes_entry(r[wanted_node->second->protocol_nodes_key()], n); + r[wanted_node->second->protocol_nodes_key()] = write_nodes_entry(n); } } diff --git a/src/kademlia/node_id.cpp b/src/kademlia/node_id.cpp index 3966f47f4..e59ed30f0 100644 --- a/src/kademlia/node_id.cpp +++ b/src/kademlia/node_id.cpp @@ -198,9 +198,9 @@ node_id generate_id(address const& ip) return generate_id_impl(ip, random(0xffffffff)); } -bool matching_prefix(node_entry const& n, int mask, int prefix, int offset) +bool matching_prefix(node_id const& nid, int mask, int prefix, int offset) { - node_id id = n.id; + node_id id = nid; id <<= offset; return (id[0] & mask) == prefix; } diff --git a/src/kademlia/routing_table.cpp b/src/kademlia/routing_table.cpp index 88ff230fd..c29904b68 100644 --- a/src/kademlia/routing_table.cpp +++ b/src/kademlia/routing_table.cpp @@ -740,7 +740,7 @@ ip_ok: for (j = b.begin(); j != b.end(); ++j) { - if (!matching_prefix(*j, mask, candidate_prefix, prefix_offset)) continue; + if (!matching_prefix(j->id, mask, candidate_prefix, prefix_offset)) continue; nodes.push_back(j); } } diff --git a/test/test_dht.cpp b/test/test_dht.cpp index 7550a2c1d..3f942e0b8 100644 --- a/test/test_dht.cpp +++ b/test/test_dht.cpp @@ -140,9 +140,10 @@ void lazy_from_entry(entry const& e, bdecode_node& l) TEST_CHECK(ret == 0); } -void write_peers(entry::dictionary_type& r, std::set const& peers) +entry write_peers(std::set const& peers) { - entry::list_type& pe = r["values"].list(); + entry r; + entry::list_type& pe = r.list(); for (auto const& p : peers) { std::string endpoint(18, '\0'); @@ -151,6 +152,7 @@ void write_peers(entry::dictionary_type& r, std::set const& peers endpoint.resize(out - endpoint.begin()); pe.push_back(entry(endpoint)); } + return r; } struct msg_args @@ -200,14 +202,14 @@ struct msg_args msg_args& want(std::string w) { a["want"].list().push_back(w); return *this; } - msg_args& nodes(nodes_t const& n) - { if (!n.empty()) dht::write_nodes_entry(a["nodes"], n); return *this; } + msg_args& nodes(std::vector const& n) + { if (!n.empty()) a["nodes"] = dht::write_nodes_entry(n); return *this; } - msg_args& nodes6(nodes_t const& n) - { if (!n.empty()) dht::write_nodes_entry(a["nodes6"], n); return *this; } + msg_args& nodes6(std::vector const& n) + { if (!n.empty()) a["nodes6"] = dht::write_nodes_entry(n); return *this; } msg_args& peers(std::set const& p) - { if (!p.empty()) write_peers(a.dict(), p); return *this; } + { if (!p.empty()) a.dict()["values"] = write_peers(p); return *this; } entry a; }; @@ -1754,7 +1756,7 @@ void test_bootstrap(address(&rand_addr)()) } udp::endpoint found_node(rand_addr(), 2235); - nodes_t nodes; + std::vector nodes; nodes.push_back(found_node); g_sent_packets.clear(); if (initial_node.address().is_v4()) @@ -1858,7 +1860,7 @@ void test_short_nodes(address(&rand_addr)()) } udp::endpoint found_node(rand_addr(), 2235); - nodes_t nodes; + std::vector nodes; nodes.push_back(found_node); g_sent_packets.clear(); msg_args args; @@ -1949,7 +1951,7 @@ void test_get_peers(address(&rand_addr)()) peers[0].insert(tcp::endpoint(rand_addr(), 4113)); udp::endpoint next_node(rand_addr(), 2235); - nodes_t nodes; + std::vector nodes; nodes.push_back(next_node); g_sent_packets.clear(); @@ -2250,7 +2252,7 @@ TORRENT_TEST(immutable_put) std::snprintf(tok, sizeof(tok), "%02d", i); msg_args args; - args.token(tok).port(1234).nid(nodes[i].id).nodes(nodes_t(1, nodes[i])); + args.token(tok).port(1234).nid(nodes[i].id).nodes({nodes[i]}); send_dht_response(t.dht_node, response, nodes[i].ep(), args); g_sent_packets.erase(packet); } @@ -2352,7 +2354,7 @@ TORRENT_TEST(mutable_put) std::snprintf(tok, sizeof(tok), "%02d", i); msg_args args; - args.token(tok).port(1234).nid(nodes[i].id).nodes(nodes_t(1, nodes[i])); + args.token(tok).port(1234).nid(nodes[i].id).nodes({nodes[i]}); send_dht_response(t.dht_node, response, nodes[i].ep(), args); g_sent_packets.erase(packet); } @@ -2472,7 +2474,7 @@ TORRENT_TEST(traversal_done) // add the address of the closest node to the first response if (i == 1) - args.nodes(nodes_t(1, nodes[8])); + args.nodes({nodes[8]}); send_dht_response(t.dht_node, response, nodes[i].ep(), args); g_sent_packets.erase(packet);