Refactor: remove recursionDepth from DHT lookups

llarp/dht/context.cpp

@@ -45,16 +45,14 @@ namespace llarp
         GetRouter()->rcLookupHandler().CheckRC(rc);
       }
 
-      /// on behalf of whoasked request introset for target from dht router with
-      /// key askpeer
       void
-      LookupIntroSetRecursive(
+      LookupIntroSetRelayed(
           const Key_t& target, const Key_t& whoasked, uint64_t whoaskedTX,
-          const Key_t& askpeer, uint64_t recursionDepth, uint64_t relayOrder,
+          const Key_t& askpeer, uint64_t relayOrder,
           service::EncryptedIntroSetLookupHandler result = nullptr) override;
 
       void
-      LookupIntroSetIterative(
+      LookupIntroSetDirect(
           const Key_t& target, const Key_t& whoasked, uint64_t whoaskedTX,
           const Key_t& askpeer,
           service::EncryptedIntroSetLookupHandler result = nullptr) override;
@@ -94,7 +92,6 @@ namespace llarp
       void
       LookupIntroSetForPath(const Key_t& addr, uint64_t txid,
                             const llarp::PathID_t& path, const Key_t& askpeer,
-                            uint64_t recursionDepth,
                             uint64_t relayOrder) override;
 
       /// send a dht message to peer, if keepalive is true then keep the session
@@ -543,15 +540,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 relayOrder)
+                                   uint64_t relayOrder)
     {
       TXOwner asker(OurKey(), txid);
       TXOwner peer(askpeer, ++ids);
       _pendingIntrosetLookups.NewTX(
           peer, asker, addr,
           new LocalServiceAddressLookup(path, txid, relayOrder, addr, this,
-                                        askpeer),
+                                        askpeer));
-          ((recursionDepth + 1) * 2000));
     }
 
     void
@@ -569,18 +565,16 @@ namespace llarp
     }
 
     void
-    Context::LookupIntroSetRecursive(
+    Context::LookupIntroSetRelayed(
         const Key_t& addr, const Key_t& whoasked, uint64_t txid,
-        const Key_t& askpeer, uint64_t recursionDepth, uint64_t relayOrder,
+        const Key_t& askpeer, uint64_t relayOrder,
         service::EncryptedIntroSetLookupHandler handler)
     {
       TXOwner asker(whoasked, txid);
       TXOwner peer(askpeer, ++ids);
       _pendingIntrosetLookups.NewTX(
           peer, asker, addr,
-          new ServiceAddressLookup(asker, addr, this, recursionDepth,
+          new ServiceAddressLookup(asker, addr, this, relayOrder, handler));
-                                   relayOrder, handler),
-          ((recursionDepth + 1) * 2000));
     }
 
     void
@@ -594,7 +588,7 @@ namespace llarp
     }
 
     void
-    Context::LookupIntroSetIterative(
+    Context::LookupIntroSetDirect(
         const Key_t& addr, const Key_t& whoasked, uint64_t txid,
         const Key_t& askpeer, service::EncryptedIntroSetLookupHandler handler)
     {
@@ -602,7 +596,7 @@ namespace llarp
       TXOwner peer(askpeer, ++ids);
       _pendingIntrosetLookups.NewTX(
           peer, asker, addr,
-          new ServiceAddressLookup(asker, addr, this, 0, 0, handler), 1000);
+          new ServiceAddressLookup(asker, addr, this, 0, handler), 1000);
     }
 
     bool

llarp/dht/context.hpp

@@ -42,17 +42,17 @@ namespace llarp
                             uint64_t whoaskedTX, const Key_t& askpeer,
                             RouterLookupHandler result = nullptr) = 0;
 
-      /// on behalf of whoasked request introset for target from dht router with
+      /// Ask a Service Node to perform an Introset lookup for us
-      /// key askpeer
       virtual void
-      LookupIntroSetRecursive(
+      LookupIntroSetRelayed(
           const Key_t& target, const Key_t& whoasked, uint64_t whoaskedTX,
-          const Key_t& askpeer, uint64_t recursionDepth, uint64_t relayOrder,
+          const Key_t& askpeer, uint64_t relayOrder,
           service::EncryptedIntroSetLookupHandler result =
               service::EncryptedIntroSetLookupHandler()) = 0;
 
+      /// Directly as a Service Node for an Introset
       virtual void
-      LookupIntroSetIterative(
+      LookupIntroSetDirect(
           const Key_t& target, const Key_t& whoasked, uint64_t whoaskedTX,
           const Key_t& askpeer,
           service::EncryptedIntroSetLookupHandler result =
@@ -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, uint64_t relayOrder) = 0;
+                            uint64_t relayOrder) = 0;
 
       virtual void
       DHTSendTo(const RouterID& peer, IMessage* msg, bool keepalive = true) = 0;

llarp/dht/localserviceaddresslookup.cpp

@@ -15,7 +15,7 @@ namespace llarp
         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,
+        : ServiceAddressLookup(TXOwner{ctx->OurKey(), txid}, addr, ctx, 
                                relayOrder, nullptr)
         , localPath(pathid)
     {

llarp/dht/messages/findintro.cpp

@@ -9,8 +9,6 @@ namespace llarp
 {
   namespace dht
   {
-    /// 2 ** 12 which is 4096 nodes, after which this starts to fail "more"
-    const uint64_t FindIntroMessage::MaxRecursionDepth = 12;
     FindIntroMessage::~FindIntroMessage()              = default;
 
     bool
@@ -24,9 +22,6 @@ namespace llarp
       if(!BEncodeMaybeReadDictInt("O", relayOrder, read, k, val))
         return false;
 
-      if(!BEncodeMaybeReadDictInt("R", recursionDepth, read, k, val))
-        return false;
-
       if(!BEncodeMaybeReadDictEntry("S", location, read, k, val))
         return false;
 
@@ -55,9 +50,6 @@ namespace llarp
         if(!BEncodeWriteDictInt("O", relayOrder, buf))
           return false;
 
-        // recursion
-        if(!BEncodeWriteDictInt("R", recursionDepth, buf))
-          return false;
         // service address
         if(!BEncodeWriteDictEntry("S", location, buf))
           return false;
@@ -70,10 +62,6 @@ namespace llarp
         // relay order
         if(!BEncodeWriteDictInt("O", relayOrder, buf))
           return false;
-
-        // recursion
-        if(!BEncodeWriteDictInt("R", recursionDepth, buf))
-          return false;
       }
       // txid
       if(!BEncodeWriteDictInt("T", txID, buf))
@@ -89,12 +77,6 @@ namespace llarp
     FindIntroMessage::HandleMessage(
         llarp_dht_context* ctx, std::vector< IMessage::Ptr_t >& replies) const
     {
-      if(recursionDepth > MaxRecursionDepth)
-      {
-        llarp::LogError("recursion depth big, ", recursionDepth, "> ",
-                        MaxRecursionDepth);
-        return false;
-      }
       auto& dht = *ctx->impl;
       if(dht.pendingIntrosetLookups().HasPendingLookupFrom(TXOwner{From, txID}))
       {
@@ -116,18 +98,6 @@ namespace llarp
       // we are relaying this message for e.g. a client
       if(relayed)
       {
-
-        // sanity check -- shouldn't happen
-        if(recursionDepth == 0)
-        {
-          // TODO: we're effectively calling this an illegal state, if we
-          //       support (relayed == true && recursionDepth == 0) then we need
-          //       to actually look this up here
-          LogWarn("Got FIM with relayed == true and recursionDepth == 0");
-          replies.emplace_back(new GotIntroMessage({}, txID));
-          return true;
-        }
-
         uint32_t numDesired = 0;
         if(relayOrder == 0)
           numDesired = 2;
@@ -155,52 +125,21 @@ namespace llarp
         for(const auto& entry : closestRCs)
         {
           Key_t peer = Key_t(entry.pubkey);
-          dht.LookupIntroSetForPath(location, txID, pathID, peer,
+          dht.LookupIntroSetForPath(location, txID, pathID, peer, 0);
-                                    recursionDepth - 1, 0);
         }
       }
       else
       {
-
         // we should have this value if introset was propagated properly
         const auto maybe = dht.GetIntroSetByLocation(location);
         if(maybe.has_value())
         {
           replies.emplace_back(new GotIntroMessage({maybe.value()}, txID));
-          return true;
-        }
-
-        if (recursionDepth == 0)
-        {
-          LogWarn("Got FIM with relayed == false and recursionDepth == 0 but we don't have introset");
-          replies.emplace_back(new GotIntroMessage({}, txID));
-          return true;
         }
         else
         {
-          // TODO: this entire else-block should now be dead code...
+          LogWarn("Got FIM with relayed == false and we don't have entry");
-          LogWarn("Got FIM with relayed == false and recursionDepth > 0");
+          replies.emplace_back(new GotIntroMessage({}, txID));
-
-          const auto rc = dht.GetRouter()->nodedb()->FindClosestTo(location);
-
-          Key_t peer = Key_t(rc.pubkey);
-          const Key_t& us = dht.OurKey();
-
-          if((us ^ location) <= (peer ^ location))
-          {
-            // ask next closest as we are recursive
-            // TODO: this code path should be dead code with our redundant DHT strategy
-            std::set< Key_t > exclude = {us, From};
-            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, 0);
         }
       }
       return true;

llarp/dht/messages/findintro.hpp

@@ -12,8 +12,6 @@ namespace llarp
   {
     struct FindIntroMessage final : public IMessage
     {
-      static const uint64_t MaxRecursionDepth;
-      uint64_t recursionDepth = 0;
       Key_t location;
       llarp::service::Tag tagName;
       uint64_t txID       = 0;
@@ -27,20 +25,14 @@ namespace llarp
         relayOrder = order;
       }
 
-      FindIntroMessage(const llarp::service::Tag& tag, uint64_t txid,
+      FindIntroMessage(const llarp::service::Tag& tag, uint64_t txid)
-                       bool iterate = true)
           : IMessage({}), tagName(tag), txID(txid)
       {
-        if(iterate)
-          recursionDepth = 0;
-        else
-          recursionDepth = 1;
       }
 
       explicit FindIntroMessage(uint64_t txid, const Key_t& addr,
-                                uint64_t maxRecursionDepth, uint64_t order)
+                                uint64_t order)
           : IMessage({})
-          , recursionDepth(maxRecursionDepth)
           , location(addr)
           , txID(txid)
           , relayOrder(order)

llarp/dht/serviceaddresslookup.cpp

@@ -11,11 +11,9 @@ namespace llarp
   {
     ServiceAddressLookup::ServiceAddressLookup(
         const TXOwner &asker, const Key_t &addr, AbstractContext *ctx,
-        uint64_t recursion, uint32_t order,
+        uint32_t order, service::EncryptedIntroSetLookupHandler handler)
-        service::EncryptedIntroSetLookupHandler handler)
         : TX< Key_t, service::EncryptedIntroSet >(asker, addr, ctx)
         , handleResult(std::move(handler))
-        , recursionDepth(recursion)
         , relayOrder(order)
     {
       peersAsked.insert(ctx->OurKey());
@@ -56,22 +54,14 @@ namespace llarp
     {
       parent->DHTSendTo(
           peer.node.as_array(),
-          new FindIntroMessage(peer.txid, target, recursionDepth, relayOrder));
+          new FindIntroMessage(peer.txid, target, relayOrder));
     }
 
     void
     ServiceAddressLookup::DoNextRequest(const Key_t &ask)
     {
-      if(recursionDepth)
+      (void)ask;
-      {
+      // do nothing -- we handle propagating this explicitly
-        parent->LookupIntroSetRecursive(target, whoasked.node, whoasked.txid,
-                                        ask, recursionDepth - 1, relayOrder);
-      }
-      else
-      {
-        parent->LookupIntroSetIterative(target, whoasked.node, whoasked.txid,
-                                        ask);
-      }
     }
 
     void

llarp/dht/serviceaddresslookup.hpp

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

llarp/dht/taglookup.cpp

@@ -30,7 +30,7 @@ namespace llarp
     {
       parent->DHTSendTo(
           peer.node.as_array(),
-          new FindIntroMessage(target, peer.txid, recursionDepth));
+          new FindIntroMessage(target, peer.txid));
     }
 
     void

llarp/service/hidden_service_address_lookup.cpp

@@ -46,7 +46,7 @@ namespace llarp
     {
       auto msg = std::make_shared< routing::DHTMessage >();
       msg->M.emplace_back(std::make_unique< dht::FindIntroMessage >(
-          txid, location, 2, relayOrder));
+          txid, location, relayOrder));
       return msg;
     }
 

test/dht/mock_context.hpp

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