Merge pull request #3592

90a16b1 crypto: fix initialization order issue with random mutex (moneromooo-monero)
6a61f52 unit_tests: add ringdb unit tests (moneromooo-monero)

6 files changed, 196 insertions, 15 deletions

diff --git a/src/crypto/crypto.cpp b/src/crypto/crypto.cpp
index ba0149240..f4ef751d3 100644
--- a/src/crypto/crypto.cpp
+++ b/src/crypto/crypto.cpp
@@ -70,8 +70,6 @@ namespace crypto {
 #include "random.h"
   }
 
-  boost::mutex random_lock;
-
   static inline unsigned char *operator &(ec_point &point) {
     return &reinterpret_cast<unsigned char &>(point);
   }
@@ -88,6 +86,13 @@ namespace crypto {
     return &reinterpret_cast<const unsigned char &>(scalar);
   }
 
+  void generate_random_bytes_thread_safe(size_t N, uint8_t *bytes)
+  {
+    static boost::mutex random_lock;
+    boost::lock_guard<boost::mutex> lock(random_lock);
+    generate_random_bytes_not_thread_safe(N, bytes);
+  }
+
   /* generate a random 32-byte (256-bit) integer and copy it to res */
   static inline void random_scalar_not_thread_safe(ec_scalar &res) {
     unsigned char tmp[64];
@@ -96,8 +101,10 @@ namespace crypto {
     memcpy(&res, tmp, 32);
   }
   static inline void random_scalar(ec_scalar &res) {
-    boost::lock_guard<boost::mutex> lock(random_lock);
-    random_scalar_not_thread_safe(res);
+    unsigned char tmp[64];
+    generate_random_bytes_thread_safe(64, tmp);
+    sc_reduce(tmp);
+    memcpy(&res, tmp, 32);
   }
 
   void hash_to_scalar(const void *data, size_t length, ec_scalar &res) {
diff --git a/src/crypto/crypto.h b/src/crypto/crypto.h
index 81ebfb9e2..9ea0f2ec0 100644
--- a/src/crypto/crypto.h
+++ b/src/crypto/crypto.h
@@ -53,8 +53,6 @@ namespace crypto {
 #include "random.h"
   }
 
-  extern boost::mutex random_lock;
-
 #pragma pack(push, 1)
   POD_CLASS ec_point {
     char data[32];
@@ -149,11 +147,12 @@ namespace crypto {
       const public_key *const *, std::size_t, const signature *);
   };
 
+  void generate_random_bytes_thread_safe(size_t N, uint8_t *bytes);
+
   /* Generate N random bytes
    */
   inline void rand(size_t N, uint8_t *bytes) {
-    boost::lock_guard<boost::mutex> lock(random_lock);
-    generate_random_bytes_not_thread_safe(N, bytes);
+    generate_random_bytes_thread_safe(N, bytes);
   }
 
   /* Generate a value filled with random bytes.
@@ -161,8 +160,7 @@ namespace crypto {
   template<typename T>
   typename std::enable_if<std::is_pod<T>::value, T>::type rand() {
     typename std::remove_cv<T>::type res;
-    boost::lock_guard<boost::mutex> lock(random_lock);
-    generate_random_bytes_not_thread_safe(sizeof(T), &res);
+    generate_random_bytes_thread_safe(sizeof(T), (uint8_t*)&res);
     return res;
   }
 
diff --git a/src/wallet/ringdb.cpp b/src/wallet/ringdb.cpp
index 44992520f..3f2634c8b 100644
--- a/src/wallet/ringdb.cpp
+++ b/src/wallet/ringdb.cpp
@@ -190,7 +190,8 @@ namespace tools
 {
 
 ringdb::ringdb(std::string filename, const std::string &genesis):
-  filename(filename)
+  filename(filename),
+  env(NULL)
 {
   MDB_txn *txn;
   bool tx_active = false;
@@ -227,9 +228,18 @@ ringdb::ringdb(std::string filename, const std::string &genesis):
 
 ringdb::~ringdb()
 {
-  mdb_dbi_close(env, dbi_rings);
-  mdb_dbi_close(env, dbi_blackballs);
-  mdb_env_close(env);
+  close();
+}
+
+void ringdb::close()
+{
+  if (env)
+  {
+    mdb_dbi_close(env, dbi_rings);
+    mdb_dbi_close(env, dbi_blackballs);
+    mdb_env_close(env);
+    env = NULL;
+  }
 }
 
 bool ringdb::add_rings(const crypto::chacha_key &chacha_key, const cryptonote::transaction_prefix &tx)
diff --git a/src/wallet/ringdb.h b/src/wallet/ringdb.h
index 2bd1ac149..6b4bce124 100644
--- a/src/wallet/ringdb.h
+++ b/src/wallet/ringdb.h
@@ -41,6 +41,7 @@ namespace tools
   {
   public:
     ringdb(std::string filename, const std::string &genesis);
+    void close();
     ~ringdb();
 
     bool add_rings(const crypto::chacha_key &chacha_key, const cryptonote::transaction_prefix &tx);
diff --git a/tests/unit_tests/CMakeLists.txt b/tests/unit_tests/CMakeLists.txt
index 249dbdbe8..8cc074bb2 100644
--- a/tests/unit_tests/CMakeLists.txt
+++ b/tests/unit_tests/CMakeLists.txt
@@ -68,7 +68,8 @@ set(unit_tests_sources
   varint.cpp
   ringct.cpp
   output_selection.cpp
-  vercmp.cpp)
+  vercmp.cpp
+  ringdb.cpp)
 
 set(unit_tests_headers
   unit_tests_utils.h)
diff --git a/tests/unit_tests/ringdb.cpp b/tests/unit_tests/ringdb.cpp
new file mode 100644
index 000000000..d50d61b0f
--- /dev/null
+++ b/tests/unit_tests/ringdb.cpp
@@ -0,0 +1,164 @@
+// Copyright (c) 2018, The Monero Project
+// 
+// All rights reserved.
+// 
+// Redistribution and use in source and binary forms, with or without modification, are
+// permitted provided that the following conditions are met:
+// 
+// 1. Redistributions of source code must retain the above copyright notice, this list of
+//    conditions and the following disclaimer.
+// 
+// 2. Redistributions in binary form must reproduce the above copyright notice, this list
+//    of conditions and the following disclaimer in the documentation and/or other
+//    materials provided with the distribution.
+// 
+// 3. Neither the name of the copyright holder nor the names of its contributors may be
+//    used to endorse or promote products derived from this software without specific
+//    prior written permission.
+// 
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY
+// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
+// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+// THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+// STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <string.h>
+#include <boost/filesystem.hpp>
+
+#include "gtest/gtest.h"
+
+#include "string_tools.h"
+#include "crypto/crypto.h"
+#include "crypto/random.h"
+#include "crypto/chacha.h"
+#include "wallet/ringdb.h"
+
+static crypto::chacha_key generate_chacha_key()
+{
+  uint8_t key[CHACHA_KEY_SIZE];
+  crypto::rand(CHACHA_KEY_SIZE, key);
+  crypto::chacha_key chacha_key;
+  memcpy(&chacha_key, key, CHACHA_KEY_SIZE);
+  return chacha_key;
+}
+
+static crypto::key_image generate_key_image()
+{
+  return crypto::rand<crypto::key_image>();
+}
+
+static crypto::public_key generate_output()
+{
+  return crypto::rand<crypto::public_key>();
+}
+
+
+static const crypto::chacha_key KEY_1 = generate_chacha_key();
+static const crypto::chacha_key KEY_2 = generate_chacha_key();
+static const crypto::key_image KEY_IMAGE_1 = generate_key_image();
+static const crypto::public_key OUTPUT_1 = generate_output();
+static const crypto::public_key OUTPUT_2 = generate_output();
+
+class RingDB: public tools::ringdb
+{
+public:
+  RingDB(const char *genesis = ""): tools::ringdb(make_filename(), genesis) { }
+  ~RingDB() { close(); boost::filesystem::remove_all(filename); free(filename); }
+
+private:
+  std::string make_filename()
+  {
+    boost::filesystem::path path = tools::get_default_data_dir();
+    path /= "fake";
+#if defined(__MINGW32__) || defined(__MINGW__)
+    filename = tempnam(path.string().c_str(), "ringdb-test-");
+    EXPECT_TRUE(filename != NULL);
+#else
+    path /= "ringdb-test-XXXXXX";
+    filename = strdup(path.string().c_str());
+    EXPECT_TRUE(mkdtemp(filename) != NULL);
+#endif
+    return filename;
+  }
+
+private:
+  char *filename;
+};
+
+TEST(ringdb, not_found)
+{
+  RingDB ringdb;
+  std::vector<uint64_t> outs;
+  ASSERT_FALSE(ringdb.get_ring(KEY_1, KEY_IMAGE_1, outs));
+}
+
+TEST(ringdb, found)
+{
+  RingDB ringdb;
+  std::vector<uint64_t> outs, outs2;
+  outs.push_back(43); outs.push_back(7320); outs.push_back(8429);
+  ASSERT_TRUE(ringdb.set_ring(KEY_1, KEY_IMAGE_1, outs, false));
+  ASSERT_TRUE(ringdb.get_ring(KEY_1, KEY_IMAGE_1, outs2));
+  ASSERT_EQ(outs, outs2);
+}
+
+TEST(ringdb, convert)
+{
+  RingDB ringdb;
+  std::vector<uint64_t> outs, outs2;
+  outs.push_back(43); outs.push_back(7320); outs.push_back(8429);
+  ASSERT_TRUE(ringdb.set_ring(KEY_1, KEY_IMAGE_1, outs, true));
+  ASSERT_TRUE(ringdb.get_ring(KEY_1, KEY_IMAGE_1, outs2));
+  ASSERT_EQ(outs2.size(), 3);
+  ASSERT_EQ(outs2[0], 43);
+  ASSERT_EQ(outs2[1], 43+7320);
+  ASSERT_EQ(outs2[2], 43+7320+8429);
+}
+
+TEST(ringdb, different_genesis)
+{
+  RingDB ringdb;
+  std::vector<uint64_t> outs, outs2;
+  outs.push_back(43); outs.push_back(7320); outs.push_back(8429);
+  ASSERT_TRUE(ringdb.set_ring(KEY_1, KEY_IMAGE_1, outs, false));
+  ASSERT_FALSE(ringdb.get_ring(KEY_2, KEY_IMAGE_1, outs2));
+}
+
+TEST(blackball, not_found)
+{
+  RingDB ringdb;
+  ASSERT_TRUE(ringdb.blackball(OUTPUT_1));
+  ASSERT_FALSE(ringdb.blackballed(OUTPUT_2));
+}
+
+TEST(blackball, found)
+{
+  RingDB ringdb;
+  ASSERT_TRUE(ringdb.blackball(OUTPUT_1));
+  ASSERT_TRUE(ringdb.blackballed(OUTPUT_1));
+}
+
+TEST(blackball, unblackball)
+{
+  RingDB ringdb;
+  ASSERT_TRUE(ringdb.blackball(OUTPUT_1));
+  ASSERT_TRUE(ringdb.unblackball(OUTPUT_1));
+  ASSERT_FALSE(ringdb.blackballed(OUTPUT_1));
+}
+
+TEST(blackball, clear)
+{
+  RingDB ringdb;
+  ASSERT_TRUE(ringdb.blackball(OUTPUT_1));
+  ASSERT_TRUE(ringdb.clear_blackballs());
+  ASSERT_FALSE(ringdb.blackballed(OUTPUT_1));
+}
+