Merge pull request #1091 from majestrate/redundant-dht-lookups-2020-02-10

Redundant dht lookups

Makefile

@@ -178,6 +178,8 @@ release-compile: release-configure
 $(TARGETS): release-compile
 
 release: $(TARGETS)
+	make -C '$(BUILD_ROOT)' test
+	make -C '$(BUILD_ROOT)' check
 
 shadow-configure: clean
 	mkdir -p $(BUILD_ROOT)
@@ -214,9 +216,12 @@ testnet:
 
 $(TEST_EXE): debug
 
-test: $(TEST_EXE)
+gtest: $(TEST_EXE)
 	test x$(CROSS) = xOFF && $(TEST_EXE) || test x$(CROSS) = xON
 
+test: gtest
+	$(MAKE) -C $(BUILD_ROOT) check
+
 static-configure: $(LIBUV_PREFIX) $(LIBCURL_PREFIX)
 	(test x$(TOOLCHAIN) = x && $(CONFIG_CMD) -DCMAKE_BUILD_TYPE=Release -DSTATIC_LINK=ON -DRELEASE_MOTTO="$(shell cat motto.txt)" -DCMAKE_C_FLAGS='$(CFLAGS)' -DCMAKE_CXX_FLAGS='$(CXXFLAGS)' -DLIBUV_ROOT='$(LIBUV_PREFIX)' -DLIBCURL_ROOT='$(LIBCURL_PREFIX)' ) || (test x$(TOOLCHAIN) != x && $(CONFIG_CMD) -DCMAKE_BUILD_TYPE=Release -DSTATIC_LINK=ON -DRELEASE_MOTTO="$(shell cat motto.txt)" -DCMAKE_C_FLAGS='$(CFLAGS)' -DCMAKE_CXX_FLAGS='$(CXXFLAGS)' -DLIBUV_ROOT='$(LIBUV_PREFIX)' -DLIBCURL_ROOT='$(LIBCURL_PREFIX)' -DCMAKE_TOOLCHAIN_FILE=$(TOOLCHAIN) -DNATIVE_BUILD=OFF )
 

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 R,
+          const Key_t& askpeer, uint64_t recursionDepth, uint64_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 R) override;
+                            uint64_t recursionDepth,
+                            uint64_t relayOrder) override;
 
       /// send a dht message to peer, if keepalive is true then keep the session
       /// with that peer alive for 10 seconds
@@ -498,7 +499,7 @@ namespace llarp
       llarp::LogDebug("initialize dht with key ", ourKey);
       // start exploring
 
-      r->logic()->call_later(
+      router->logic()->call_later(
           exploreInterval,
           std::bind(&llarp::dht::Context::handle_explore_timer, this,
                     exploreInterval));
@@ -541,14 +542,16 @@ namespace llarp
     void
     Context::LookupIntroSetForPath(const Key_t& addr, uint64_t txid,
                                    const llarp::PathID_t& path,
-                                   const Key_t& askpeer, uint64_t R)
+                                   const Key_t& askpeer,
+                                   uint64_t recursionDepth, uint64_t relayOrder)
     {
       TXOwner asker(OurKey(), txid);
       TXOwner peer(askpeer, ++ids);
       _pendingIntrosetLookups.NewTX(
           peer, asker, addr,
-          new LocalServiceAddressLookup(path, txid, addr, this, askpeer),
-          ((R + 1) * 2000));
+          new LocalServiceAddressLookup(path, txid, relayOrder, addr, this,
+                                        askpeer),
+          ((recursionDepth + 1) * 2000));
     }
 
     void
@@ -568,15 +571,16 @@ namespace llarp
     void
     Context::LookupIntroSetRecursive(
         const Key_t& addr, const Key_t& whoasked, uint64_t txid,
-        const Key_t& askpeer, uint64_t R,
+        const Key_t& askpeer, uint64_t recursionDepth, uint64_t relayOrder,
         service::EncryptedIntroSetLookupHandler handler)
     {
       TXOwner asker(whoasked, txid);
       TXOwner peer(askpeer, ++ids);
       _pendingIntrosetLookups.NewTX(
           peer, asker, addr,
-          new ServiceAddressLookup(asker, addr, this, R, handler),
-          ((R + 1) * 2000));
+          new ServiceAddressLookup(asker, addr, this, recursionDepth,
+                                   relayOrder, handler),
+          ((recursionDepth + 1) * 2000));
     }
 
     void
@@ -598,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
@@ -636,11 +640,11 @@ namespace llarp
       }
       for(const auto& f : foundRouters)
       {
-        const RouterID r = f.as_array();
+        const RouterID id = f.as_array();
         // discard shit routers
-        if(router->routerProfiling().IsBadForConnect(r))
+        if(router->routerProfiling().IsBadForConnect(id))
           continue;
-        closer.emplace_back(r);
+        closer.emplace_back(id);
       }
       llarp::LogDebug("Gave ", closer.size(), " routers for exploration");
       reply.emplace_back(new GotRouterMessage(txid, closer, false));

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 R,
+          const Key_t& askpeer, uint64_t recursionDepth, uint64_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 R) = 0;
+                            uint64_t recursionDepth, uint64_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, uint64_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,
+                                uint64_t relayOrder, const Key_t &addr,
+                                AbstractContext *ctx,
                                 __attribute__((unused)) const Key_t &askpeer);
 
       void

llarp/dht/message.cpp

@@ -47,7 +47,7 @@ namespace llarp
           switch(*strbuf.base)
           {
             case 'F':
-              msg = std::make_unique< FindIntroMessage >(From, relayed);
+              msg = std::make_unique< FindIntroMessage >(From, relayed, 0);
               break;
             case 'R':
               if(relayed)

llarp/dht/messages/findintro.cpp

@@ -21,6 +21,9 @@ namespace llarp
       if(!BEncodeMaybeReadDictEntry("N", tagName, read, k, val))
         return false;
 
+      if(!BEncodeMaybeReadDictInt("O", relayOrder, read, k, val))
+        return false;
+
       if(!BEncodeMaybeReadDictInt("R", recursionDepth, read, k, val))
         return false;
 
@@ -48,6 +51,10 @@ namespace llarp
         return false;
       if(tagName.Empty())
       {
+        // relay order
+        if(!BEncodeWriteDictInt("O", relayOrder, buf))
+          return false;
+
         // recursion
         if(!BEncodeWriteDictInt("R", recursionDepth, buf))
           return false;
@@ -59,6 +66,11 @@ namespace llarp
       {
         if(!BEncodeWriteDictEntry("N", tagName, buf))
           return false;
+
+        // relay order
+        if(!BEncodeWriteDictInt("O", relayOrder, buf))
+          return false;
+
         // recursion
         if(!BEncodeWriteDictInt("R", recursionDepth, buf))
           return false;
@@ -117,29 +129,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;
+      uint64_t relayOrder = 0;
 
-      FindIntroMessage(const Key_t& from, bool relay) : IMessage(from)
+      FindIntroMessage(const Key_t& from, bool relay, uint64_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, uint64_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 r, service::EncryptedIntroSetLookupHandler handler)
+        uint64_t recursion, uint32_t order,
+        service::EncryptedIntroSetLookupHandler handler)
         : TX< Key_t, service::EncryptedIntroSet >(asker, addr, ctx)
         , handleResult(std::move(handler))
-        , R(r)
+        , recursionDepth(recursion)
+        , relayOrder(order)
     {
       peersAsked.insert(ctx->OurKey());
     }
@@ -52,17 +54,18 @@ namespace llarp
     void
     ServiceAddressLookup::Start(const TXOwner &peer)
     {
-      parent->DHTSendTo(peer.node.as_array(),
-                        new FindIntroMessage(peer.txid, target, R));
+      parent->DHTSendTo(
+          peer.node.as_array(),
+          new FindIntroMessage(peer.txid, target, recursionDepth, relayOrder));
     }
 
     void
     ServiceAddressLookup::DoNextRequest(const Key_t &ask)
     {
-      if(R)
+      if(recursionDepth)
       {
         parent->LookupIntroSetRecursive(target, whoasked.node, whoasked.txid,
-                                        ask, R - 1);
+                                        ask, recursionDepth - 1, relayOrder);
       }
       else
       {

llarp/dht/serviceaddresslookup.hpp

@@ -15,10 +15,12 @@ namespace llarp
     struct ServiceAddressLookup : public TX< Key_t, service::EncryptedIntroSet >
     {
       service::EncryptedIntroSetLookupHandler handleResult;
-      uint64_t R;
+      uint64_t recursionDepth;
+      uint32_t relayOrder;
 
       ServiceAddressLookup(const TXOwner &asker, const Key_t &addr,
-                           AbstractContext *ctx, uint64_t r,
+                           AbstractContext *ctx, uint64_t recursionDepth,
+                           uint32_t relayOrder,
                            service::EncryptedIntroSetLookupHandler handler);
 
       bool

llarp/dht/taglookup.cpp

@@ -28,8 +28,9 @@ namespace llarp
     void
     TagLookup::Start(const TXOwner &peer)
     {
-      parent->DHTSendTo(peer.node.as_array(),
-                        new FindIntroMessage(target, peer.txid, R));
+      parent->DHTSendTo(
+          peer.node.as_array(),
+          new FindIntroMessage(target, peer.txid, recursionDepth));
     }
 
     void

llarp/dht/taglookup.hpp

@@ -11,11 +11,11 @@ namespace llarp
   {
     struct TagLookup : public TX< service::Tag, service::EncryptedIntroSet >
     {
-      uint64_t R;
+      uint64_t recursionDepth;
       TagLookup(const TXOwner &asker, const service::Tag &tag,
-                AbstractContext *ctx, uint64_t r)
+                AbstractContext *ctx, uint64_t recursion)
           : TX< service::Tag, service::EncryptedIntroSet >(asker, tag, ctx)
-          , R(r)
+          , recursionDepth(recursion)
       {
       }
 

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,32 @@ 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)
+{
+  llarp::util::Lock lock(&access);
+  std::vector< const llarp::RouterContact * > all;
+
+  all.reserve(entries.size());
+  for(auto &entry : entries)
+  {
+    all.push_back(&entry.second.rc);
+  }
+
+  auto it_mid = numRouters < all.size() ? all.begin() + numRouters : all.end();
+  std::partial_sort(all.begin(), it_mid, all.end(),
+                    [compare = llarp::dht::XorMetric{location}](
+                        auto *a, auto *b) { return compare(*a, *b); });
+
+  std::vector< llarp::RouterContact > closest;
+  closest.reserve(numRouters);
+  for(auto it = all.begin(); it != it_mid; ++it)
+    closest.push_back(**it);
+
+  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/path/path.cpp

@@ -50,7 +50,8 @@ namespace llarp
       // initialize parts of the introduction
       intro.router = hops[hsz - 1].rc.pubkey;
       intro.pathID = hops[hsz - 1].txID;
-      EnterState(ePathBuilding, parent->Now());
+      if(parent)
+        EnterState(ePathBuilding, parent->Now());
     }
 
     bool

llarp/path/path.hpp

@@ -23,6 +23,7 @@
 #include <list>
 #include <map>
 #include <unordered_map>
+#include <unordered_set>
 #include <vector>
 
 #include <util/decaying_hashset.hpp>
@@ -127,10 +128,11 @@ namespace llarp
         }
       };
 
+      /// hash for std::shared_ptr
       struct Ptr_Hash
       {
         size_t
-        operator()(const std::shared_ptr< Path >& p) const
+        operator()(const Path_ptr& p) const
         {
           if(p == nullptr)
             return 0;
@@ -138,6 +140,32 @@ namespace llarp
         }
       };
 
+      /// hash for std::shared_ptr by path endpoint
+      struct Endpoint_Hash
+      {
+        size_t
+        operator()(const Path_ptr& p) const
+        {
+          if(p == nullptr)
+            return 0;
+          return RouterID::Hash{}(p->Endpoint());
+        }
+      };
+
+      /// comparision for equal endpoints
+      struct Endpoint_Equals
+      {
+        bool
+        operator()(const Path_ptr& left, const Path_ptr& right) const
+        {
+          return left && right && left->Endpoint() == left->Endpoint();
+        }
+      };
+
+      /// unordered set of paths with unique endpoints
+      using UniqueEndpointSet_t =
+          std::unordered_set< Path_ptr, Endpoint_Hash, Endpoint_Equals >;
+
       bool
       operator<(const Path& other) const
       {

llarp/service/endpoint.cpp

@@ -460,9 +460,20 @@ namespace llarp
     bool
     Endpoint::PublishIntroSet(const EncryptedIntroSet& i, AbstractRouter* r)
     {
-      // publish via near router
-      const auto path = GetEstablishedPathClosestTo(i.derivedSigningKey);
-      return path && PublishIntroSetVia(i, r, path);
+      /// number of routers to publish to
+      static constexpr size_t PublishRedundancy = 2;
+      const auto paths =
+          GetManyPathsWithUniqueEndpoints(this, PublishRedundancy);
+      // do publishing for each path selected
+      size_t published = 0;
+      for(const auto& path : paths)
+      {
+        if(PublishIntroSetVia(i, r, path))
+        {
+          published++;
+        }
+      }
+      return published == PublishRedundancy;
     }
 
     struct PublishIntroSetJob : public IServiceLookup
@@ -938,22 +949,9 @@ namespace llarp
 
     bool
     Endpoint::EnsurePathToService(const Address remote, PathEnsureHook hook,
-                                  ABSL_ATTRIBUTE_UNUSED llarp_time_t timeoutMS,
-                                  bool randomPath)
+                                  ABSL_ATTRIBUTE_UNUSED llarp_time_t timeoutMS)
     {
-      const dht::Key_t location = remote.ToKey();
-      path::Path_ptr path       = nullptr;
-      if(randomPath)
-        path = PickRandomEstablishedPath();
-      else
-        path = GetEstablishedPathClosestTo(location.as_array());
-      if(!path)
-      {
-        LogWarn("No outbound path for lookup yet");
-        BuildOne();
-        return false;
-      }
-
+      static constexpr size_t NumParalellLookups = 2;
       LogInfo(Name(), " Ensure Path to ", remote.ToString());
 
       auto& sessions = m_state->m_RemoteSessions;
@@ -969,23 +967,28 @@ namespace llarp
 
       auto& lookups = m_state->m_PendingServiceLookups;
 
-      if(lookups.count(remote) >= MaxConcurrentLookups)
-      {
-        path = PickRandomEstablishedPath();
-      }
+      const auto paths =
+          GetManyPathsWithUniqueEndpoints(this, NumParalellLookups);
 
       using namespace std::placeholders;
-      HiddenServiceAddressLookup* job = new HiddenServiceAddressLookup(
-          this, util::memFn(&Endpoint::OnLookup, this), location,
-          PubKey{remote.as_array()}, GenTXID());
-      LogInfo("doing lookup for ", remote, " via ", path->Endpoint());
-      if(job->SendRequestViaPath(path, Router()))
+      size_t lookedUp           = 0;
+      const dht::Key_t location = remote.ToKey();
+      for(const auto& path : paths)
       {
-        lookups.emplace(remote, hook);
-        return true;
+        HiddenServiceAddressLookup* job = new HiddenServiceAddressLookup(
+            this, util::memFn(&Endpoint::OnLookup, this), location,
+            PubKey{remote.as_array()}, 0, GenTXID());
+        LogInfo("doing lookup for ", remote, " via ", path->Endpoint(), " at ",
+                location);
+        if(job->SendRequestViaPath(path, Router()))
+        {
+          lookups.emplace(remote, hook);
+          lookedUp++;
+        }
+        else
+          LogError(Name(), " send via path failed for lookup");
       }
-      LogError("send via path failed");
-      return false;
+      return lookedUp == NumParalellLookups;
     }
 
     bool
@@ -1216,7 +1219,7 @@ namespace llarp
               [self = this](Address addr, OutboundContext* ctx) {
                 if(ctx)
                 {
-                  ctx->UpdateIntroSet(true);
+                  ctx->UpdateIntroSet();
                   for(auto& pending : self->m_state->m_PendingTraffic[addr])
                   {
                     ctx->AsyncEncryptAndSendTo(pending.Buffer(),

llarp/service/endpoint.hpp

@@ -289,7 +289,7 @@ namespace llarp
       /// address
       bool
       EnsurePathToService(const Address remote, PathEnsureHook h,
-                          uint64_t timeoutMS, bool lookupOnRandomPath = true);
+                          uint64_t timeoutMS);
 
       using SNodeEnsureHook =
           std::function< void(const RouterID, exit::BaseSession_ptr) >;

llarp/service/endpoint_util.hpp

@@ -41,6 +41,21 @@ namespace llarp
       GetConvoTagsForService(const ConvoMap& sessions, const Address& addr,
                              std::set< ConvoTag >& tags);
     };
+
+    template < typename Endpoint_t >
+    static path::Path::UniqueEndpointSet_t
+    GetManyPathsWithUniqueEndpoints(Endpoint_t* ep, size_t N, size_t tries = 10)
+    {
+      path::Path::UniqueEndpointSet_t paths;
+      do
+      {
+        --tries;
+        const auto path = ep->PickRandomEstablishedPath();
+        if(path)
+          paths.emplace(path);
+      } while(tries > 0 and paths.size() < N);
+      return paths;
+    }
   }  // namespace service
 
 }  // namespace llarp

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,
+        uint64_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;
+      uint64_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, uint64_t relayOrder,
+                                 uint64_t tx);
 
       ~HiddenServiceAddressLookup() override = default;
 

llarp/service/outbound_context.cpp

@@ -8,6 +8,8 @@
 #include <profiling.hpp>
 #include <util/meta/memfn.hpp>
 
+#include <service/endpoint_util.hpp>
+
 #include <random>
 #include <algorithm>
 
@@ -48,7 +50,7 @@ namespace llarp
         {
           SwapIntros();
         }
-        UpdateIntroSet(false);
+        UpdateIntroSet();
       }
       return true;
     }
@@ -220,38 +222,24 @@ namespace llarp
     }
 
     void
-    OutboundContext::UpdateIntroSet(bool randomizePath)
+    OutboundContext::UpdateIntroSet()
     {
       if(updatingIntroSet || markedBad)
         return;
       const auto addr = currentIntroSet.A.Addr();
 
-      path::Path_ptr path = nullptr;
-      if(randomizePath)
-      {
-        path = m_Endpoint->PickRandomEstablishedPath();
-      }
-      else
-        path = m_Endpoint->GetEstablishedPathClosestTo(addr.as_array());
-
-      if(path == nullptr)
-      {
-        path = PickRandomEstablishedPath();
-      }
-
-      if(path)
+      const auto paths    = GetManyPathsWithUniqueEndpoints(this, 2);
+      uint64_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());
-
-        updatingIntroSet = job->SendRequestViaPath(path, m_Endpoint->Router());
-      }
-      else
-      {
-        LogWarn("Cannot update introset no path for outbound session to ",
-                currentIntroSet.A.Addr().ToString());
+            location, PubKey{addr.as_array()}, relayOrder,
+            m_Endpoint->GenTXID());
+        relayOrder++;
+        if(job->SendRequestViaPath(path, m_Endpoint->Router()))
+          updatingIntroSet = true;
       }
     }
 
@@ -307,7 +295,7 @@ namespace llarp
       }
       if(currentIntroSet.HasExpiredIntros(now))
       {
-        UpdateIntroSet(false);
+        UpdateIntroSet();
       }
       // send control message if we look too quiet
       if(lastGoodSend)
@@ -419,7 +407,7 @@ namespace llarp
       {
         // update introset
         LogInfo(Name(), " updating introset");
-        UpdateIntroSet(false);
+        UpdateIntroSet();
         return true;
       }
       return false;
@@ -494,7 +482,7 @@ namespace llarp
     OutboundContext::HandlePathDied(path::Path_ptr path)
     {
       // unconditionally update introset
-      UpdateIntroSet(false);
+      UpdateIntroSet();
       const RouterID endpoint(path->Endpoint());
       // if a path to our current intro died...
       if(endpoint == remoteIntro.router)

llarp/service/outbound_context.hpp

@@ -88,7 +88,7 @@ namespace llarp
 
       /// issues a lookup to find the current intro set of the remote service
       void
-      UpdateIntroSet(bool randomizePath) override;
+      UpdateIntroSet() override;
 
       void
       HandlePathBuilt(path::Path_ptr path) override;

llarp/service/sendcontext.hpp

@@ -61,7 +61,7 @@ namespace llarp
       }
 
       virtual void
-      UpdateIntroSet(bool randomizePath = false) = 0;
+      UpdateIntroSet() = 0;
 
       virtual bool
       MarkCurrentIntroBad(llarp_time_t now) = 0;

test/CMakeLists.txt

@@ -1,4 +1,6 @@
 set(TEST_EXE testAll)
+set(CHECK_EXE checkAll)
+
 
 list(APPEND TEST_SRC
     config/test_llarp_config_config.cpp
@@ -74,3 +76,13 @@ endif(NOT WIN32)
 if(${CMAKE_SYSTEM_NAME} MATCHES "FreeBSD")
   target_link_directories(${TEST_EXE} PRIVATE /usr/local/lib)
 endif()
+
+add_executable(${CHECK_EXE}
+  nodedb/test_nodedb.cpp
+  path/test_path.cpp
+  check_main.cpp)
+
+target_link_libraries(${CHECK_EXE} PUBLIC ${STATIC_LIB})
+target_include_directories(${CHECK_EXE} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
+
+add_custom_target(check COMMAND ${CHECK_EXE})

test/check_main.cpp

@@ -0,0 +1,2 @@
+#define CATCH_CONFIG_MAIN
+#include "catch.hpp"

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, uint64_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, uint64_t));
 
       MOCK_METHOD3(DHTSendTo, void(const RouterID&, dht::IMessage*, bool));
 

test/nodedb/test_nodedb.cpp

@@ -0,0 +1,68 @@
+#include "catch.hpp"
+#include "config/config.hpp"
+
+#include <router_contact.hpp>
+#include <nodedb.hpp>
+
+TEST_CASE("FindClosestTo returns correct number of elements", "[nodedb][dht]")
+{
+
+  llarp_nodedb nodeDB(nullptr, "");
+
+  constexpr uint64_t numRCs = 3;
+  for (uint64_t i=0; i<numRCs; ++i)
+  {
+    llarp::RouterContact rc;
+    rc.pubkey[0] = i;
+    nodeDB.Insert(rc);
+  }
+
+  REQUIRE(numRCs == nodeDB.num_loaded());
+
+  llarp::dht::Key_t key;
+
+  std::vector< llarp::RouterContact > results = nodeDB.FindClosestTo(key, 4);
+
+  // we asked for more entries than nodedb had
+  REQUIRE(numRCs == results.size());
+}
+
+TEST_CASE("FindClosestTo returns properly ordered set", "[nodedb][dht]")
+{
+
+  llarp_nodedb nodeDB(nullptr, "");
+
+  // insert some RCs: a < b < c
+  llarp::RouterContact a;
+  a.pubkey[0] = 1;
+  nodeDB.Insert(a);
+
+  llarp::RouterContact b;
+  b.pubkey[0] = 2;
+  nodeDB.Insert(b);
+
+  llarp::RouterContact c;
+  c.pubkey[0] = 3;
+  nodeDB.Insert(c);
+
+  REQUIRE(3 == nodeDB.num_loaded());
+
+  llarp::dht::Key_t key;
+
+  std::vector< llarp::RouterContact > results = nodeDB.FindClosestTo(key, 2);
+  REQUIRE(2 == results.size());
+
+  // we xor'ed with 0x0, so order should be a,b,c
+  REQUIRE(a.pubkey == results[0].pubkey);
+  REQUIRE(b.pubkey == results[1].pubkey);
+
+  llarp::dht::Key_t compKey;
+  compKey.Fill(0xFF);
+
+  results = nodeDB.FindClosestTo(compKey, 2);
+
+  // we xor'ed with 0xF...F, so order should be inverted (c,b,a)
+  REQUIRE(c.pubkey == results[0].pubkey);
+  REQUIRE(b.pubkey == results[1].pubkey);
+}
+

test/path/test_path.cpp

@@ -0,0 +1,39 @@
+#include <path/path.hpp>
+#include "catch.hpp"
+
+using Path_t   = llarp::path::Path;
+using Path_ptr = llarp::path::Path_ptr;
+using Set_t    = llarp::path::Path::UniqueEndpointSet_t;
+using RC_t     = llarp::RouterContact;
+
+static RC_t
+MakeHop(const char name)
+{
+  RC_t rc;
+  rc.pubkey.Fill(name);
+  return rc;
+}
+
+static Path_ptr
+MakePath(std::vector< char > hops)
+{
+  std::vector< RC_t > pathHops;
+  for(const auto& hop : hops)
+    pathHops.push_back(MakeHop(hop));
+  return std::make_shared< Path_t >(pathHops, nullptr, 0);
+}
+
+TEST_CASE("UniqueEndpointSet_t has unique endpoints", "[path]")
+{
+  Set_t set;
+  REQUIRE(set.empty());
+  const auto inserted_first =
+      set.emplace(MakePath({'a', 'b', 'c', 'd'})).second;
+  REQUIRE(inserted_first);
+  const auto inserted_again =
+      set.emplace(MakePath({'a', 'b', 'c', 'd'})).second;
+  REQUIRE(not inserted_again);
+  const auto inserted_second =
+      set.emplace(MakePath({'d', 'c', 'b', 'a'})).second;
+  REQUIRE(inserted_second);
+}