Refactor DHT introset lookups to use redundant lookup strategy

llarp/dht/context.cpp

@@ -50,7 +50,7 @@ namespace llarp
       void
       LookupIntroSetRecursive(
           const Key_t& target, const Key_t& whoasked, uint64_t whoaskedTX,
-          const Key_t& askpeer, uint64_t recursionDepth,
+          const Key_t& askpeer, uint64_t recursionDepth, uint32_t relayOrder,
           service::EncryptedIntroSetLookupHandler result = nullptr) override;
 
       void
@@ -94,7 +94,8 @@ namespace llarp
       void
       LookupIntroSetForPath(const Key_t& addr, uint64_t txid,
                             const llarp::PathID_t& path, const Key_t& askpeer,
-                            uint64_t recursionDepth) override;
+                            uint64_t recursionDepth,
+                            uint32_t relayOrder) override;
 
       /// send a dht message to peer, if keepalive is true then keep the session
       /// with that peer alive for 10 seconds
@@ -542,13 +543,14 @@ namespace llarp
     Context::LookupIntroSetForPath(const Key_t& addr, uint64_t txid,
                                    const llarp::PathID_t& path,
                                    const Key_t& askpeer,
-                                   uint64_t recursionDepth)
+                                   uint64_t recursionDepth, uint32_t relayOrder)
     {
       TXOwner asker(OurKey(), txid);
       TXOwner peer(askpeer, ++ids);
       _pendingIntrosetLookups.NewTX(
           peer, asker, addr,
-          new LocalServiceAddressLookup(path, txid, addr, this, askpeer),
+          new LocalServiceAddressLookup(path, txid, relayOrder, addr, this,
+                                        askpeer),
           ((recursionDepth + 1) * 2000));
     }
 
@@ -569,14 +571,15 @@ namespace llarp
     void
     Context::LookupIntroSetRecursive(
         const Key_t& addr, const Key_t& whoasked, uint64_t txid,
-        const Key_t& askpeer, uint64_t recursionDepth,
+        const Key_t& askpeer, uint64_t recursionDepth, uint32_t relayOrder,
         service::EncryptedIntroSetLookupHandler handler)
     {
       TXOwner asker(whoasked, txid);
       TXOwner peer(askpeer, ++ids);
       _pendingIntrosetLookups.NewTX(
           peer, asker, addr,
-          new ServiceAddressLookup(asker, addr, this, recursionDepth, handler),
+          new ServiceAddressLookup(asker, addr, this, recursionDepth,
+                                   relayOrder, handler),
           ((recursionDepth + 1) * 2000));
     }
 
@@ -599,7 +602,7 @@ namespace llarp
       TXOwner peer(askpeer, ++ids);
       _pendingIntrosetLookups.NewTX(
           peer, asker, addr,
-          new ServiceAddressLookup(asker, addr, this, 0, handler), 1000);
+          new ServiceAddressLookup(asker, addr, this, 0, 0, handler), 1000);
     }
 
     bool

llarp/dht/context.hpp

@@ -47,7 +47,7 @@ namespace llarp
       virtual void
       LookupIntroSetRecursive(
           const Key_t& target, const Key_t& whoasked, uint64_t whoaskedTX,
-          const Key_t& askpeer, uint64_t recursionDepth,
+          const Key_t& askpeer, uint64_t recursionDepth, uint32_t relayOrder,
           service::EncryptedIntroSetLookupHandler result =
               service::EncryptedIntroSetLookupHandler()) = 0;
 
@@ -69,7 +69,7 @@ namespace llarp
       virtual void
       LookupIntroSetForPath(const Key_t& addr, uint64_t txid,
                             const PathID_t& path, const Key_t& askpeer,
-                            uint64_t recursionDepth) = 0;
+                            uint64_t recursionDepth, uint32_t relayOrder) = 0;
 
       virtual void
       DHTSendTo(const RouterID& peer, IMessage* msg, bool keepalive = true) = 0;

llarp/dht/kademlia.hpp

@@ -2,6 +2,7 @@
 #define LLARP_DHT_KADEMLIA_HPP
 
 #include <dht/key.hpp>
+#include <router_contact.hpp>
 
 namespace llarp
 {
@@ -20,6 +21,12 @@ namespace llarp
       {
         return (us ^ left) < (us ^ right);
       }
+
+      bool
+      operator()(const RouterContact& left, const RouterContact& right) const
+      {
+        return (left.pubkey ^ us) < (right.pubkey ^ us);
+      }
     };
   }  // namespace dht
 }  // namespace llarp

llarp/dht/localserviceaddresslookup.cpp

@@ -12,10 +12,11 @@ namespace llarp
   namespace dht
   {
     LocalServiceAddressLookup::LocalServiceAddressLookup(
-        const PathID_t &pathid, uint64_t txid, const Key_t &addr,
-        AbstractContext *ctx, __attribute__((unused)) const Key_t &askpeer)
-        : ServiceAddressLookup(TXOwner{ctx->OurKey(), txid}, addr, ctx, 5,
-                               nullptr)
+        const PathID_t &pathid, uint64_t txid, uint32_t relayOrder,
+        const Key_t &addr, AbstractContext *ctx,
+        __attribute__((unused)) const Key_t &askpeer)
+        : ServiceAddressLookup(TXOwner{ctx->OurKey(), txid}, addr, ctx, 2,
+                               relayOrder, nullptr)
         , localPath(pathid)
     {
     }

llarp/dht/localserviceaddresslookup.hpp

@@ -14,7 +14,8 @@ namespace llarp
       PathID_t localPath;
 
       LocalServiceAddressLookup(const PathID_t &pathid, uint64_t txid,
-                                const Key_t &addr, AbstractContext *ctx,
+                                uint32_t relayOrder, const Key_t &addr,
+                                AbstractContext *ctx,
                                 __attribute__((unused)) const Key_t &askpeer);
 
       void

llarp/dht/message.cpp

@@ -16,11 +16,12 @@ namespace llarp
     {
       const Key_t &From;
       IMessage::Ptr_t msg;
-      bool firstKey = true;
-      bool relayed  = false;
+      bool firstKey       = true;
+      bool relayed        = false;
+      uint32_t relayOrder = 0;
 
-      MessageDecoder(const Key_t &from, bool wasRelayed)
-          : From(from), relayed(wasRelayed)
+      MessageDecoder(const Key_t &from, bool wasRelayed, uint32_t order)
+          : From(from), relayed(wasRelayed), relayOrder(order)
       {
       }
 
@@ -47,7 +48,8 @@ namespace llarp
           switch(*strbuf.base)
           {
             case 'F':
-              msg = std::make_unique< FindIntroMessage >(From, relayed);
+              msg = std::make_unique< FindIntroMessage >(From, relayed,
+                                                         relayOrder);
               break;
             case 'R':
               if(relayed)
@@ -86,9 +88,10 @@ namespace llarp
     };
 
     IMessage::Ptr_t
-    DecodeMesssage(const Key_t &from, llarp_buffer_t *buf, bool relayed)
+    DecodeMesssage(const Key_t &from, llarp_buffer_t *buf, bool relayed,
+                   uint32_t relayOrder)
     {
-      MessageDecoder dec(from, relayed);
+      MessageDecoder dec(from, relayed, relayOrder);
       if(!bencode_read_dict(dec, buf))
         return nullptr;
 
@@ -97,13 +100,14 @@ namespace llarp
 
     struct ListDecoder
     {
-      ListDecoder(bool hasRelayed, const Key_t &from,
+      ListDecoder(bool hasRelayed, uint32_t order, const Key_t &from,
                   std::vector< IMessage::Ptr_t > &list)
-          : relayed(hasRelayed), From(from), l(list)
+          : relayed(hasRelayed), relayOrder(order), From(from), l(list)
       {
       }
 
       bool relayed;
+      uint32_t relayOrder;
       const Key_t &From;
       std::vector< IMessage::Ptr_t > &l;
 
@@ -112,7 +116,7 @@ namespace llarp
       {
         if(!has)
           return true;
-        auto msg = DecodeMesssage(From, buffer, relayed);
+        auto msg = DecodeMesssage(From, buffer, relayed, relayOrder);
         if(msg)
         {
           l.emplace_back(std::move(msg));
@@ -125,9 +129,10 @@ namespace llarp
 
     bool
     DecodeMesssageList(Key_t from, llarp_buffer_t *buf,
-                       std::vector< IMessage::Ptr_t > &list, bool relayed)
+                       std::vector< IMessage::Ptr_t > &list, bool relayed,
+                       uint32_t relayOrder)
     {
-      ListDecoder dec(relayed, from, list);
+      ListDecoder dec(relayed, relayOrder, from, list);
       return bencode_read_list(dec, buf);
     }
   }  // namespace dht

llarp/dht/message.hpp

@@ -41,12 +41,13 @@ namespace llarp
     };
 
     IMessage::Ptr_t
-    DecodeMessage(const Key_t& from, llarp_buffer_t* buf, bool relayed = false);
+    DecodeMessage(const Key_t& from, llarp_buffer_t* buf, bool relayed = false,
+                  uint32_t relayOrder = 0);
 
     bool
     DecodeMesssageList(Key_t from, llarp_buffer_t* buf,
                        std::vector< IMessage::Ptr_t >& dst,
-                       bool relayed = false);
+                       bool relayed = false, uint32_t relayOrder = 0);
   }  // namespace dht
 }  // namespace llarp
 

llarp/dht/messages/findintro.cpp

@@ -117,29 +117,58 @@ namespace llarp
       }
 
       // we are recursive
-      const auto rc = dht.GetRouter()->nodedb()->FindClosestTo(location);
+      if(relayed)
+      {
+        uint32_t numDesired = 0;
+        if(relayOrder == 0)
+          numDesired = 2;
+        else if(relayOrder == 1)
+          numDesired = 4;
+        else
+        {
+          // TODO: consider forward-compatibility here
+          LogError("Error: relayOrder must be 0 or 1");
+          return false;
+        }
 
-      Key_t peer = Key_t(rc.pubkey);
+        auto closestRCs =
+            dht.GetRouter()->nodedb()->FindClosestTo(location, numDesired);
 
-      if((us ^ location) <= (peer ^ location))
-      {
-        // ask second closest as we are recursive
-        if(not dht.Nodes()->FindCloseExcluding(location, peer, exclude))
+        // if relayOrder == 1, we want the 3rd and 4th closest, so remove the
+        // 1st and 2nd closest
+        if(relayOrder == 1)
         {
-          // no second closeset
-          replies.emplace_back(new GotIntroMessage({}, txID));
-          return true;
+          auto itr = closestRCs.begin();
+          std::advance(itr, 2);
+          closestRCs.erase(closestRCs.begin(), itr);
+        }
+
+        for(const auto& entry : closestRCs)
+        {
+          Key_t peer = Key_t(entry.pubkey);
+          dht.LookupIntroSetForPath(location, txID, pathID, peer,
+                                    recursionDepth - 1, 0);
         }
-      }
-      if(relayed)
-      {
-        dht.LookupIntroSetForPath(location, txID, pathID, peer,
-                                  recursionDepth - 1);
       }
       else
       {
+        const auto rc = dht.GetRouter()->nodedb()->FindClosestTo(location);
+
+        Key_t peer = Key_t(rc.pubkey);
+
+        if((us ^ location) <= (peer ^ location))
+        {
+          // ask second closest as we are recursive
+          if(not dht.Nodes()->FindCloseExcluding(location, peer, exclude))
+          {
+            // no second closeset
+            replies.emplace_back(new GotIntroMessage({}, txID));
+            return true;
+          }
+        }
+
         dht.LookupIntroSetRecursive(location, From, txID, peer,
-                                    recursionDepth - 1);
+                                    recursionDepth - 1, 0);
       }
       return true;
     }

llarp/dht/messages/findintro.hpp

@@ -16,12 +16,15 @@ namespace llarp
       uint64_t recursionDepth = 0;
       Key_t location;
       llarp::service::Tag tagName;
-      uint64_t txID = 0;
-      bool relayed  = false;
+      uint64_t txID       = 0;
+      bool relayed        = false;
+      uint32_t relayOrder = 0;
 
-      FindIntroMessage(const Key_t& from, bool relay) : IMessage(from)
+      FindIntroMessage(const Key_t& from, bool relay, uint32_t order)
+          : IMessage(from)
       {
-        relayed = relay;
+        relayed    = relay;
+        relayOrder = order;
       }
 
       FindIntroMessage(const llarp::service::Tag& tag, uint64_t txid,
@@ -35,11 +38,12 @@ namespace llarp
       }
 
       explicit FindIntroMessage(uint64_t txid, const Key_t& addr,
-                                uint64_t maxRecursionDepth)
+                                uint64_t maxRecursionDepth, uint32_t order)
           : IMessage({})
           , recursionDepth(maxRecursionDepth)
           , location(addr)
           , txID(txid)
+          , relayOrder(order)
       {
         tagName.Zero();
       }

llarp/dht/serviceaddresslookup.cpp

@@ -11,10 +11,12 @@ namespace llarp
   {
     ServiceAddressLookup::ServiceAddressLookup(
         const TXOwner &asker, const Key_t &addr, AbstractContext *ctx,
-        uint64_t recursion, service::EncryptedIntroSetLookupHandler handler)
+        uint64_t recursion, uint32_t order,
+        service::EncryptedIntroSetLookupHandler handler)
         : TX< Key_t, service::EncryptedIntroSet >(asker, addr, ctx)
         , handleResult(std::move(handler))
         , recursionDepth(recursion)
+        , relayOrder(order)
     {
       peersAsked.insert(ctx->OurKey());
     }
@@ -54,7 +56,7 @@ namespace llarp
     {
       parent->DHTSendTo(
           peer.node.as_array(),
-          new FindIntroMessage(peer.txid, target, recursionDepth));
+          new FindIntroMessage(peer.txid, target, recursionDepth, relayOrder));
     }
 
     void
@@ -63,7 +65,7 @@ namespace llarp
       if(recursionDepth)
       {
         parent->LookupIntroSetRecursive(target, whoasked.node, whoasked.txid,
-                                        ask, recursionDepth - 1);
+                                        ask, recursionDepth - 1, relayOrder);
       }
       else
       {

llarp/dht/serviceaddresslookup.hpp

@@ -16,9 +16,11 @@ namespace llarp
     {
       service::EncryptedIntroSetLookupHandler handleResult;
       uint64_t recursionDepth;
+      uint32_t relayOrder;
 
       ServiceAddressLookup(const TXOwner &asker, const Key_t &addr,
                            AbstractContext *ctx, uint64_t recursionDepth,
+                           uint32_t relayOrder,
                            service::EncryptedIntroSetLookupHandler handler);
 
       bool

llarp/nodedb.cpp

@@ -1,6 +1,7 @@
 #include <nodedb.hpp>
 
 #include <crypto/crypto.hpp>
+#include <crypto/types.hpp>
 #include <router_contact.hpp>
 #include <util/buffer.hpp>
 #include <util/encode.hpp>
@@ -11,6 +12,7 @@
 #include <util/thread/thread_pool.hpp>
 #include <dht/kademlia.hpp>
 
+#include <algorithm>
 #include <fstream>
 #include <unordered_map>
 #include <utility>
@@ -112,6 +114,89 @@ llarp_nodedb::FindClosestTo(const llarp::dht::Key_t &location)
   return rc;
 }
 
+std::vector< llarp::RouterContact >
+llarp_nodedb::FindClosestTo(const llarp::dht::Key_t &location,
+                            uint32_t numRouters)
+{
+  if(numRouters < num_loaded())
+  {
+    numRouters = num_loaded();
+  }
+  /*
+   * XXX: this attempts to use std::partial_sort_copy(), which has a few
+  requirements
+   * we can't quite meet:
+   *
+   * 1) the iterators must operate on same types (NetDBMap_t would give us
+  pairs)
+   * 2) the second set of iterators must be of types which:
+   *     - swap is defined
+   *     - are move-constructible
+   *     - are move-assignable
+   *    std::pair is none (or at least not all) of these things
+   * 3) the result iterators must specify a range; so their container must not
+  be empty
+   *
+
+  using NetDBPair_t = std::pair< llarp::RouterID, NetDBEntry >;
+
+  struct Compare
+  {
+    const llarp::dht::Key_t target;
+
+    Compare(const llarp::dht::Key_t& target_) : target(target_)
+    {
+    }
+
+    bool
+    operator()(const NetDBPair_t& left, const NetDBPair_t& right) const
+    {
+      return (left.second.rc.pubkey ^ target) < (right.second.rc.pubkey ^
+  target);
+    }
+  };
+
+  llarp::util::Lock lock(&access);
+
+  std::vector< NetDBPair_t > closestEntries;
+  closestEntries.resize(numRouters); // so that we have a valid iterator range
+
+  const Compare compare(location);
+
+  std::partial_sort_copy(
+      entries.begin(), entries.end(),
+      closestEntries.begin(), closestEntries.end(),
+      compare);
+
+  std::vector< llarp::RouterContact > closest;
+  closest.reserve(closestEntries.size());
+
+  for (const auto& entry : closestEntries) {
+    closest.push_back(entry.second.rc);
+  }
+
+  return closest;
+  */
+
+  // TODO: avoid this ugly copy
+  std::vector< llarp::RouterContact > all;
+  all.reserve(entries.size());
+  for(auto &entry : entries)
+  {
+    all.push_back(entry.second.rc);
+  }
+
+  std::vector< llarp::RouterContact > closest;
+  closest.resize(numRouters);
+
+  const llarp::dht::XorMetric compare(location);
+
+  std::partial_sort_copy(all.begin(), all.end(), closest.begin(), closest.end(),
+                         compare);
+
+  return closest;
+}
+
 /// skiplist directory is hex encoded first nibble
 /// skiplist filename is <base32encoded>.snode.signed
 std::string

llarp/nodedb.hpp

@@ -77,6 +77,10 @@ struct llarp_nodedb
   llarp::RouterContact
   FindClosestTo(const llarp::dht::Key_t &location);
 
+  /// find the $numRouters closest routers to the given DHT key
+  std::vector< llarp::RouterContact >
+  FindClosestTo(const llarp::dht::Key_t &location, uint32_t numRouters);
+
   /// return true if we should save our nodedb to disk
   bool
   ShouldSaveToDisk(llarp_time_t now = 0) const;

llarp/service/endpoint.cpp

@@ -978,7 +978,7 @@ namespace llarp
       {
         HiddenServiceAddressLookup* job = new HiddenServiceAddressLookup(
             this, util::memFn(&Endpoint::OnLookup, this), location,
-            PubKey{remote.as_array()}, GenTXID());
+            PubKey{remote.as_array()}, 0, GenTXID());
         LogInfo("doing lookup for ", remote, " via ", path->Endpoint(), " at ",
                 location);
         if(job->SendRequestViaPath(path, Router()))

llarp/service/hidden_service_address_lookup.cpp

@@ -8,13 +8,12 @@ namespace llarp
 {
   namespace service
   {
-    HiddenServiceAddressLookup::HiddenServiceAddressLookup(Endpoint* p,
-                                                           HandlerFunc h,
-                                                           const dht::Key_t& l,
-                                                           const PubKey& k,
-                                                           uint64_t tx)
+    HiddenServiceAddressLookup::HiddenServiceAddressLookup(
+        Endpoint* p, HandlerFunc h, const dht::Key_t& l, const PubKey& k,
+        uint32_t order, uint64_t tx)
         : IServiceLookup(p, tx, "HSLookup")
         , rootkey(k)
+        , relayOrder(order)
         , location(l)
         , handle(std::move(h))
     {
@@ -46,8 +45,8 @@ namespace llarp
     HiddenServiceAddressLookup::BuildRequestMessage()
     {
       auto msg = std::make_shared< routing::DHTMessage >();
-      msg->M.emplace_back(
-          std::make_unique< dht::FindIntroMessage >(txid, location, 2));
+      msg->M.emplace_back(std::make_unique< dht::FindIntroMessage >(
+          txid, location, 2, relayOrder));
       return msg;
     }
 

llarp/service/hidden_service_address_lookup.hpp

@@ -13,6 +13,7 @@ namespace llarp
     struct HiddenServiceAddressLookup : public IServiceLookup
     {
       const PubKey rootkey;
+      uint32_t relayOrder;
       const dht::Key_t location;
       using HandlerFunc = std::function< bool(
           const Address&, absl::optional< const IntroSet >, const RouterID&) >;
@@ -20,7 +21,8 @@ namespace llarp
 
       HiddenServiceAddressLookup(Endpoint* p, HandlerFunc h,
                                  const dht::Key_t& location,
-                                 const PubKey& rootkey, uint64_t tx);
+                                 const PubKey& rootkey, uint32_t relayOrder,
+                                 uint64_t tx);
 
       ~HiddenServiceAddressLookup() override = default;
 

llarp/service/outbound_context.cpp

@@ -228,13 +228,16 @@ namespace llarp
         return;
       const auto addr = currentIntroSet.A.Addr();
 
-      const auto paths = GetManyPathsWithUniqueEndpoints(this, 2);
+      const auto paths    = GetManyPathsWithUniqueEndpoints(this, 2);
+      uint32_t relayOrder = 0;
       for(const auto& path : paths)
       {
         HiddenServiceAddressLookup* job = new HiddenServiceAddressLookup(
             m_Endpoint,
             util::memFn(&OutboundContext::OnIntroSetUpdate, shared_from_this()),
-            location, PubKey{addr.as_array()}, m_Endpoint->GenTXID());
+            location, PubKey{addr.as_array()}, relayOrder,
+            m_Endpoint->GenTXID());
+        relayOrder++;
         if(job->SendRequestViaPath(path, m_Endpoint->Router()))
           updatingIntroSet = true;
       }

test/dht/mock_context.hpp

@@ -19,9 +19,9 @@ namespace llarp
                    void(const RouterID&, const dht::Key_t&, uint64_t,
                         const dht::Key_t&, RouterLookupHandler));
 
-      MOCK_METHOD6(LookupIntroSetRecursive,
+      MOCK_METHOD7(LookupIntroSetRecursive,
                    void(const dht::Key_t&, const dht::Key_t&, uint64_t,
-                        const dht::Key_t&, uint64_t,
+                        const dht::Key_t&, uint64_t, uint32_t,
                         service::EncryptedIntroSetLookupHandler));
 
       MOCK_METHOD5(LookupIntroSetIterative,
@@ -35,9 +35,9 @@ namespace llarp
                    void(const RouterID& target, uint64_t txid,
                         const PathID_t& path, const dht::Key_t& askpeer));
 
-      MOCK_METHOD5(LookupIntroSetForPath,
+      MOCK_METHOD6(LookupIntroSetForPath,
                    void(const dht::Key_t&, uint64_t, const PathID_t&,
-                        const dht::Key_t&, uint64_t));
+                        const dht::Key_t&, uint64_t, uint32_t));
 
       MOCK_METHOD3(DHTSendTo, void(const RouterID&, dht::IMessage*, bool));