More unit tests of dht internals

CMakeLists.txt

@@ -622,13 +622,16 @@ set(DNS_SRC
 set(TEST_SRC
   # helpers
   test/main.cpp
+  test/dht/mock_context.cpp
   test/test_util.cpp
   # actual test cases
   test/crypto/test_llarp_crypto_types.cpp
   test/crypto/test_llarp_crypto.cpp
   test/dht/test_llarp_dht_bucket.cpp
+  test/dht/test_llarp_dht_explorenetworkjob.cpp
   test/dht/test_llarp_dht_kademlia.cpp
   test/dht/test_llarp_dht_key.cpp
+  test/dht/test_llarp_dht_tx.cpp
   test/dht/test_llarp_dht_txowner.cpp
   test/dns/test_llarp_dns_dns.cpp
   test/exit/test_llarp_exit_context.cpp

llarp/crypto/types.hpp

@@ -58,6 +58,24 @@ namespace llarp
     }
   };
 
+  inline bool
+  operator==(const PubKey &lhs, const PubKey &rhs)
+  {
+    return lhs.as_array() == rhs.as_array();
+  }
+
+  inline bool
+  operator==(const PubKey &lhs, const RouterID &rhs)
+  {
+    return lhs.as_array() == rhs.as_array();
+  }
+
+  inline bool
+  operator==(const RouterID &lhs, const PubKey &rhs)
+  {
+    return lhs.as_array() == rhs.as_array();
+  }
+
   struct SecretKey final : public AlignedBuffer< SECKEYSIZE >
   {
     SecretKey() : AlignedBuffer< SECKEYSIZE >(){};

llarp/dht/explorenetworkjob.cpp

@@ -18,13 +18,14 @@ namespace llarp
     ExploreNetworkJob::SendReply()
     {
       llarp::LogInfo("got ", valuesFound.size(), " routers from exploration");
+
+      auto router = parent->GetRouter();
+      using std::placeholders::_1;
       for(const auto &pk : valuesFound)
       {
         // lookup router
         parent->LookupRouter(
-            pk,
-            std::bind(&Router::HandleDHTLookupForExplore, parent->GetRouter(), pk,
-                      std::placeholders::_1));
+            pk, std::bind(&Router::HandleDHTLookupForExplore, router, pk, _1));
       }
     }
   }  // namespace dht

llarp/dht/tx.hpp

@@ -10,8 +10,6 @@
 
 namespace llarp
 {
-  struct Router;
-
   namespace dht
   {
     struct AbstractContext;
@@ -32,12 +30,16 @@ namespace llarp
 
       virtual ~TX(){};
 
+      void
+      OnFound(const Key_t& askedPeer, const V& value);
+
+      /// return true if we want to persist this tx
+      bool
+      AskNextPeer(const Key_t& prevPeer, const std::unique_ptr< Key_t >& next);
+
       virtual bool
       Validate(const V& value) const = 0;
 
-      void
-      OnFound(const Key_t askedPeer, const V& value);
-
       virtual void
       Start(const TXOwner& peer) = 0;
 
@@ -47,17 +49,13 @@ namespace llarp
       virtual void
       DoNextRequest(const Key_t& peer) = 0;
 
-      /// return true if we want to persist this tx
-      bool
-      AskNextPeer(const Key_t& prevPeer, const std::unique_ptr< Key_t >& next);
-
       virtual void
       SendReply() = 0;
     };
 
     template < typename K, typename V >
     inline void
-    TX< K, V >::OnFound(const Key_t askedPeer, const V& value)
+    TX< K, V >::OnFound(const Key_t& askedPeer, const V& value)
     {
       peersAsked.insert(askedPeer);
       if(Validate(value))

llarp/router_id.hpp

@@ -44,6 +44,13 @@ namespace llarp
 
     using Hash = AlignedBuffer< SIZE >::Hash;
   };
+
+  inline bool
+  operator==(const RouterID& lhs, const RouterID& rhs)
+  {
+    return lhs.as_array() == rhs.as_array();
+  }
+
 }  // namespace llarp
 
 #endif

test/dht/mock_context.cpp

@@ -0,0 +1 @@
+#include <dht/mock_context.hpp>

test/dht/mock_context.hpp

@@ -0,0 +1,46 @@
+#ifndef TEST_LLARP_MOCK_CONTEXT
+#define TEST_LLARP_MOCK_CONTEXT
+
+#include <dht/context.hpp>
+
+#include <gmock/gmock.h>
+
+namespace llarp
+{
+  namespace test
+  {
+    struct MockContext final : public dht::AbstractContext
+    {
+      MOCK_METHOD2(LookupRouter, bool(const RouterID&, RouterLookupHandler));
+
+      MOCK_METHOD6(LookupIntroSetRecursive,
+                   void(const service::Address&, const dht::Key_t&, uint64_t,
+                        const dht::Key_t&, uint64_t,
+                        service::IntroSetLookupHandler));
+
+      MOCK_METHOD5(LookupIntroSetIterative,
+                   void(const service::Address&, const dht::Key_t&, uint64_t,
+                        const dht::Key_t&, service::IntroSetLookupHandler));
+
+      MOCK_METHOD3(
+          FindRandomIntroSetsWithTagExcluding,
+          std::set< service::IntroSet >(const service::Tag&, size_t,
+                                        const std::set< service::IntroSet >&));
+
+      MOCK_METHOD3(DHTSendTo, void(const RouterID&, dht::IMessage*, bool));
+
+      MOCK_CONST_METHOD0(Now, llarp_time_t());
+
+      MOCK_CONST_METHOD0(Crypto, llarp::Crypto*());
+
+      MOCK_CONST_METHOD0(GetRouter, llarp::Router*());
+
+      MOCK_CONST_METHOD0(OurKey, const dht::Key_t&());
+
+      MOCK_CONST_METHOD0(Nodes, dht::Bucket< dht::RCNode >*());
+    };
+
+  }  // namespace test
+}  // namespace llarp
+
+#endif

test/dht/test_llarp_dht_explorenetworkjob.cpp

@@ -0,0 +1,105 @@
+#include <dht/explorenetworkjob.hpp>
+
+#include <dht/messages/findrouter.hpp>
+#include <dht/mock_context.hpp>
+#include <test_util.hpp>
+
+#include <gtest/gtest.h>
+
+using namespace llarp;
+using namespace ::testing;
+
+using test::makeBuf;
+
+struct TestDhtExploreNetworkJob : public ::testing::Test
+{
+  RouterID peer;
+  test::MockContext context;
+  dht::ExploreNetworkJob exploreNetworkJob;
+
+  TestDhtExploreNetworkJob()
+      : peer(makeBuf< RouterID >(0x01)), exploreNetworkJob(peer, &context)
+  {
+  }
+};
+
+TEST_F(TestDhtExploreNetworkJob, validate)
+{
+  const RouterID other = makeBuf< RouterID >(0x02);
+  ASSERT_TRUE(exploreNetworkJob.Validate(other));
+}
+
+TEST_F(TestDhtExploreNetworkJob, get_next_peer)
+{
+  dht::Key_t key = makeBuf< dht::Key_t >(0x02);
+  std::set< dht::Key_t > exclude;
+  ASSERT_FALSE(exploreNetworkJob.GetNextPeer(key, exclude));
+}
+
+TEST_F(TestDhtExploreNetworkJob, do_next)
+{
+  const dht::Key_t key = makeBuf< dht::Key_t >(0x02);
+  ASSERT_NO_THROW(exploreNetworkJob.DoNextRequest(key));
+}
+
+TEST_F(TestDhtExploreNetworkJob, start)
+{
+  // Verify input arguments are passed correctly.
+  // The actual logic is inside the `dht::AbstractContext` implementation.
+
+  const auto txKey = makeBuf< dht::Key_t >(0x02);
+  uint64_t txId    = 4;
+
+  dht::TXOwner txOwner(txKey, txId);
+
+  // clang-format off
+  EXPECT_CALL(context, DHTSendTo(
+    Eq(txKey.as_array()),
+    WhenDynamicCastTo< dht::FindRouterMessage* >(NotNull()),
+    true)
+  ).Times(1);
+  // clang-format off
+
+  ASSERT_NO_THROW(exploreNetworkJob.Start(txOwner));
+}
+
+TEST_F(TestDhtExploreNetworkJob, send_reply)
+{
+  // Concerns:
+  // - Empty collection
+  // - Lookup router fails (returns false)
+  // - Number of calls matches collection size
+
+  {
+    exploreNetworkJob.valuesFound.clear();
+    EXPECT_CALL(context, LookupRouter(_, _)).Times(0);
+    EXPECT_CALL(context, GetRouter()).WillOnce(Return(nullptr));
+
+    ASSERT_NO_THROW(exploreNetworkJob.SendReply());
+  }
+
+  {
+    exploreNetworkJob.valuesFound.clear();
+    exploreNetworkJob.valuesFound.push_back(makeBuf<RouterID>(0x00));
+    exploreNetworkJob.valuesFound.push_back(makeBuf<RouterID>(0x01));
+    exploreNetworkJob.valuesFound.push_back(makeBuf<RouterID>(0x02));
+
+    EXPECT_CALL(context, GetRouter()).WillOnce(Return(nullptr));
+    EXPECT_CALL(context, LookupRouter(Ne(makeBuf<RouterID>(0x01)), _)).Times(2).WillRepeatedly(Return(true));
+    EXPECT_CALL(context, LookupRouter(Eq(makeBuf<RouterID>(0x01)), _)).WillOnce(Return(false));
+
+    ASSERT_NO_THROW(exploreNetworkJob.SendReply());
+  }
+
+  {
+    exploreNetworkJob.valuesFound.clear();
+    exploreNetworkJob.valuesFound.push_back(makeBuf<RouterID>(0x00));
+    exploreNetworkJob.valuesFound.push_back(makeBuf<RouterID>(0x01));
+    exploreNetworkJob.valuesFound.push_back(makeBuf<RouterID>(0x02));
+
+    EXPECT_CALL(context, GetRouter()).WillOnce(Return(nullptr));
+    EXPECT_CALL(context, LookupRouter(_, _)).Times(3).WillRepeatedly(Return(true));
+
+    ASSERT_NO_THROW(exploreNetworkJob.SendReply());
+  }
+}

test/dht/test_llarp_dht_tx.cpp

@@ -0,0 +1,170 @@
+#include <dht/tx.hpp>
+
+#include <test_util.hpp>
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+using namespace llarp;
+using namespace ::testing;
+
+using llarp::test::makeBuf;
+
+// Mock implementation of TX.
+struct TestTx final : public dht::TX< dht::Key_t, std::string >
+{
+  TestTx(const dht::TXOwner& asker, const dht::Key_t& k,
+         dht::AbstractContext* p)
+      : dht::TX< dht::Key_t, std::string >(asker, k, p)
+  {
+  }
+
+  MOCK_CONST_METHOD1(Validate, bool(const std::string&));
+
+  MOCK_METHOD1(Start, void(const dht::TXOwner&));
+
+  MOCK_METHOD2(GetNextPeer, bool(dht::Key_t&, const std::set< dht::Key_t >&));
+
+  MOCK_METHOD1(DoNextRequest, void(const dht::Key_t&));
+
+  MOCK_METHOD0(SendReply, void());
+};
+
+struct TestDhtTx : public Test
+{
+  dht::TXOwner asker;
+  dht::Key_t key;
+  TestTx tx;
+
+  TestDhtTx() : tx(asker, key, nullptr)
+  {
+  }
+};
+
+TEST_F(TestDhtTx, on_found)
+{
+  // Concerns
+  // - Validate returns true
+  // - Repeated call on success
+  // - Validate returns false
+  // - Repeated call on failure
+  // - Repeated call on success after failure
+
+  const auto key = makeBuf< dht::Key_t >(0x00);
+  std::string val("good value");
+
+  // Validate returns true
+  {
+    EXPECT_CALL(tx, Validate(val)).WillOnce(Return(true));
+
+    tx.OnFound(key, val);
+
+    ASSERT_THAT(tx.peersAsked, Contains(key));
+    ASSERT_THAT(tx.valuesFound, Contains(val));
+  }
+
+  // Repeated call on success
+  {
+    EXPECT_CALL(tx, Validate(val)).WillOnce(Return(true));
+    tx.OnFound(key, val);
+    ASSERT_THAT(tx.peersAsked, Contains(key));
+    ASSERT_THAT(tx.valuesFound, Contains(val));
+  }
+
+  const auto key1 = makeBuf< dht::Key_t >(0x01);
+  std::string badVal("bad value");
+
+  // Validate returns false
+  {
+    EXPECT_CALL(tx, Validate(badVal)).WillOnce(Return(false));
+
+    tx.OnFound(key1, badVal);
+
+    ASSERT_THAT(tx.peersAsked, Contains(key1));
+    ASSERT_THAT(tx.valuesFound, Not(Contains(badVal)));
+  }
+
+  // Repeated call on failure
+  {
+    EXPECT_CALL(tx, Validate(badVal)).WillOnce(Return(false));
+
+    tx.OnFound(key1, badVal);
+
+    ASSERT_THAT(tx.peersAsked, Contains(key1));
+    ASSERT_THAT(tx.valuesFound, Not(Contains(badVal)));
+  }
+
+  // Repeated call on success after failure
+  {
+    EXPECT_CALL(tx, Validate(badVal)).WillOnce(Return(true));
+
+    tx.OnFound(key1, badVal);
+
+    ASSERT_THAT(tx.peersAsked, Contains(key1));
+    ASSERT_THAT(tx.valuesFound, Contains(badVal));
+  }
+}
+
+TEST_F(TestDhtTx, ask_next_peer)
+{
+  // Concerns:
+  // - GetNextPeer fails
+  // - Next Peer is not closer
+  // - next ptr is null
+  // - next ptr is not null
+
+  const auto key0 = makeBuf< dht::Key_t >(0x00);
+  const auto key1 = makeBuf< dht::Key_t >(0x01);
+  const auto key2 = makeBuf< dht::Key_t >(0x02);
+
+  {
+    // GetNextPeer fails
+    EXPECT_CALL(tx, GetNextPeer(_, _)).WillOnce(Return(false));
+
+    EXPECT_CALL(tx, DoNextRequest(key1)).Times(0);
+
+    ASSERT_FALSE(tx.AskNextPeer(key0, {}));
+    ASSERT_THAT(tx.peersAsked, Contains(key0));
+
+    tx.peersAsked.clear();
+  }
+
+  {
+    // Next Peer is not closer
+    EXPECT_CALL(tx, GetNextPeer(_, _))
+        .WillOnce(DoAll(SetArgReferee< 0 >(key1), Return(true)));
+
+    EXPECT_CALL(tx, DoNextRequest(key1)).Times(0);
+
+    ASSERT_FALSE(tx.AskNextPeer(key0, {}));
+    ASSERT_THAT(tx.peersAsked, Contains(key0));
+
+    tx.peersAsked.clear();
+  }
+
+  {
+    // next ptr is null
+    EXPECT_CALL(tx, GetNextPeer(_, _))
+        .WillOnce(DoAll(SetArgReferee< 0 >(key1), Return(true)));
+
+    EXPECT_CALL(tx, DoNextRequest(key1)).Times(1);
+
+    ASSERT_TRUE(tx.AskNextPeer(key2, {}));
+    ASSERT_THAT(tx.peersAsked, Contains(key2));
+
+    tx.peersAsked.clear();
+  }
+
+  {
+    // next ptr is not null
+    EXPECT_CALL(tx, GetNextPeer(_, _)).Times(0);
+
+    EXPECT_CALL(tx, DoNextRequest(key1)).Times(1);
+
+    auto ptr = std::make_unique< dht::Key_t >(key1);
+    ASSERT_TRUE(tx.AskNextPeer(key2, ptr));
+    ASSERT_THAT(tx.peersAsked, Contains(key2));
+
+    tx.peersAsked.clear();
+  }
+}

test/test_util.hpp

@@ -2,6 +2,7 @@
 #define TEST_UTIL_HPP
 
 #include <util/fs.hpp>
+#include <util/types.hpp>
 
 namespace llarp
 {
@@ -10,6 +11,15 @@ namespace llarp
     std::string
     randFilename();
 
+    template < typename Buf >
+    Buf
+    makeBuf(byte_t val)
+    {
+      Buf b;
+      b.Fill(val);
+      return b;
+    }
+
     struct FileGuard
     {
       const fs::path &p;