Merge #1629: [DB] Bitcoin 0.12-0.14 dbwrapper improvements

82f9088 Refactor: Remove using namespace <xxx> from /dbwrapper_tests (random-zebra)
6db0b37 rpc: make `gettxoutsettinfo` run lock-free (random-zebra)
f009cf4 Do not shadow members in dbwrapper (random-zebra)
b7e540c dbwrapper: Move `HandleError` to `dbwrapper_private` (random-zebra)
43004d0 leveldbwrapper file rename to dbwrapper.* (random-zebra)
c882dd9 leveldbwrapper symbol rename: Remove "Level" from class, etc. names (random-zebra)
f6496da leveldbwrapper: Remove unused .Prev(), .SeekToLast() methods (random-zebra)
cacf3c2 Fix chainstate serialized_size computation (random-zebra)
a2a3d33 Add tests for CLevelDBBatch, CLevelDBIterator (random-zebra)
94150ac Encapsulate CLevelDB iterators cleanly (random-zebra)
21df7cc [DB] Refactor leveldbwrapper (random-zebra)
2251db3 change hardcoded character constants to a set of descriptive named co… (random-zebra)

Pull request description:

  This backports a series of updates and cleanups to the LevelDB wrapper from:

  - bitcoin#5707
  - bitcoin#6650 [`*`]
  - bitcoin#6777 [`*`]
  - bitcoin#6865
  - bitcoin#6873
  - bitcoin#7927 [`*`]
  - bitcoin#8467
  - bitcoin#6290
  - bitcoin#9281

  PIVX-specific edits were required to keep the sporks and zerocoin databases in line.

  [`*`] NOTE: excluding the obfuscation of databases by xoring data, as we might not want this feature (e.g. as zcash/zcash#2598). Otherwise it can be discussed, and added, with a separate PR.

ACKs for top commit:
  furszy:
    Re ACK 82f9088 .
  Fuzzbawls:
    ACK 82f9088

Tree-SHA512: 1e4a75621d2ec2eb68e01523d15321d1d2176b81aac0525617852899ab38c9b4980daecb9056d054e7961fc758a22143edf914c40d1819144a394f2869a8ad57

CMakeLists.txt

@@ -295,7 +295,7 @@ set(SERVER_SOURCES
         ./src/httprpc.cpp
         ./src/httpserver.cpp
         ./src/init.cpp
-        ./src/leveldbwrapper.cpp
+        ./src/dbwrapper.cpp
         ./src/main.cpp
         ./src/merkleblock.cpp
         ./src/miner.cpp

src/Makefile.am

@@ -127,7 +127,7 @@ BITCOIN_CORE_H = \
   key.h \
   key_io.h \
   keystore.h \
-  leveldbwrapper.h \
+  dbwrapper.h \
   limitedmap.h \
   logging.h \
   main.h \
@@ -230,7 +230,7 @@ libbitcoin_server_a_SOURCES = \
   httprpc.cpp \
   httpserver.cpp \
   init.cpp \
-  leveldbwrapper.cpp \
+  dbwrapper.cpp \
   main.cpp \
   merkleblock.cpp \
   miner.cpp \

src/Makefile.test.include

@@ -53,6 +53,7 @@ BITCOIN_TESTS =\
   test/getarg_tests.cpp \
   test/hash_tests.cpp \
   test/key_tests.cpp \
+  test/dbwrapper_tests.cpp \
   test/main_tests.cpp \
   test/mempool_tests.cpp \
   test/merkle_tests.cpp \

src/leveldbwrapper.cpp → src/dbwrapper.cpp

@@ -2,30 +2,19 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include "leveldbwrapper.h"
+#include "dbwrapper.h"
 
 #include "util.h"
 
 #include <boost/filesystem.hpp>
+#include <boost/scoped_ptr.hpp>
 
 #include <leveldb/cache.h>
 #include <leveldb/env.h>
 #include <leveldb/filter_policy.h>
 #include <memenv.h>
+#include <stdint.h>
 
-void HandleError(const leveldb::Status& status)
-{
-    if (status.ok())
-        return;
-    LogPrintf("%s\n", status.ToString());
-    if (status.IsCorruption())
-        throw leveldb_error("Database corrupted");
-    if (status.IsIOError())
-        throw leveldb_error("Database I/O error");
-    if (status.IsNotFound())
-        throw leveldb_error("Database entry missing");
-    throw leveldb_error("Unknown database error");
-}
 
 static leveldb::Options GetOptions(size_t nCacheSize)
 {
@@ -43,7 +32,7 @@ static leveldb::Options GetOptions(size_t nCacheSize)
     return options;
 }
 
-CLevelDBWrapper::CLevelDBWrapper(const boost::filesystem::path& path, size_t nCacheSize, bool fMemory, bool fWipe)
+CDBWrapper::CDBWrapper(const boost::filesystem::path& path, size_t nCacheSize, bool fMemory, bool fWipe)
 {
     penv = NULL;
     readoptions.verify_checksums = true;
@@ -58,17 +47,18 @@ CLevelDBWrapper::CLevelDBWrapper(const boost::filesystem::path& path, size_t nCa
     } else {
         if (fWipe) {
             LogPrintf("Wiping LevelDB in %s\n", path.string());
-            leveldb::DestroyDB(path.string(), options);
+            leveldb::Status result = leveldb::DestroyDB(path.string(), options);
+            dbwrapper_private::HandleError(result);
         }
         TryCreateDirectory(path);
         LogPrintf("Opening LevelDB in %s\n", path.string());
     }
     leveldb::Status status = leveldb::DB::Open(options, path.string(), &pdb);
-    HandleError(status);
+    dbwrapper_private::HandleError(status);
     LogPrintf("Opened LevelDB successfully\n");
 }
 
-CLevelDBWrapper::~CLevelDBWrapper()
+CDBWrapper::~CDBWrapper()
 {
     delete pdb;
     pdb = NULL;
@@ -80,9 +70,40 @@ CLevelDBWrapper::~CLevelDBWrapper()
     options.env = NULL;
 }
 
-bool CLevelDBWrapper::WriteBatch(CLevelDBBatch& batch, bool fSync)
+bool CDBWrapper::WriteBatch(CDBBatch& batch, bool fSync)
 {
     leveldb::Status status = pdb->Write(fSync ? syncoptions : writeoptions, &batch.batch);
-    HandleError(status);
+    dbwrapper_private::HandleError(status);
     return true;
 }
+
+bool CDBWrapper::IsEmpty()
+{
+    boost::scoped_ptr<CDBIterator> it(NewIterator());
+    it->SeekToFirst();
+    return !(it->Valid());
+}
+
+CDBIterator::~CDBIterator() { delete piter; }
+bool CDBIterator::Valid() { return piter->Valid(); }
+void CDBIterator::SeekToFirst() { piter->SeekToFirst(); }
+void CDBIterator::Next() { piter->Next(); }
+
+
+namespace dbwrapper_private {
+
+void HandleError(const leveldb::Status& status)
+{
+    if (status.ok())
+        return;
+    LogPrintf("%s\n", status.ToString());
+    if (status.IsCorruption())
+        throw dbwrapper_error("Database corrupted");
+    if (status.IsIOError())
+        throw dbwrapper_error("Database I/O error");
+    if (status.IsNotFound())
+        throw dbwrapper_error("Database entry missing");
+    throw dbwrapper_error("Unknown database error");
+}
+
+};

src/leveldbwrapper.h → src/dbwrapper.h

@@ -2,8 +2,8 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#ifndef BITCOIN_LEVELDBWRAPPER_H
-#define BITCOIN_LEVELDBWRAPPER_H
+#ifndef BITCOIN_DBWRAPPER_H
+#define BITCOIN_DBWRAPPER_H
 
 #include "clientversion.h"
 #include "serialize.h"
@@ -16,23 +16,35 @@
 #include <leveldb/db.h>
 #include <leveldb/write_batch.h>
 
-class leveldb_error : public std::runtime_error
+class dbwrapper_error : public std::runtime_error
 {
 public:
-    leveldb_error(const std::string& msg) : std::runtime_error(msg) {}
+    dbwrapper_error(const std::string& msg) : std::runtime_error(msg) {}
 };
 
+class CDBWrapper;
+
+/** These should be considered an implementation detail of the specific database.
+ */
+namespace dbwrapper_private {
+
+/** Handle database error by throwing dbwrapper_error exception.
+ */
 void HandleError(const leveldb::Status& status);
 
-/** Batch of changes queued to be written to a CLevelDBWrapper */
-class CLevelDBBatch
+};
+
+
+/** Batch of changes queued to be written to a CDBWrapper */
+class CDBBatch
 {
-    friend class CLevelDBWrapper;
+    friend class CDBWrapper;
 
 private:
     leveldb::WriteBatch batch;
 
 public:
+
     template <typename K, typename V>
     void Write(const K& key, const V& value)
     {
@@ -61,7 +73,65 @@ class CLevelDBBatch
     }
 };
 
-class CLevelDBWrapper
+class CDBIterator
+{
+private:
+    leveldb::Iterator *piter;
+
+public:
+    /**
+     * @param[in] _piter            The original leveldb iterator.
+     */
+    CDBIterator(leveldb::Iterator *_piter) : piter(_piter) { };
+    ~CDBIterator();
+
+    bool Valid();
+
+    void SeekToFirst();
+
+    template<typename K> void Seek(const K& key) {
+        CDataStream ssKey(SER_DISK, CLIENT_VERSION);
+        ssKey.reserve(ssKey.GetSerializeSize(key));
+        ssKey << key;
+        leveldb::Slice slKey(&ssKey[0], ssKey.size());
+        piter->Seek(slKey);
+    }
+
+    void Next();
+
+    template<typename K> bool GetKey(K& key) {
+        leveldb::Slice slKey = piter->key();
+        try {
+            CDataStream ssKey(slKey.data(), slKey.data() + slKey.size(), SER_DISK, CLIENT_VERSION);
+            ssKey >> key;
+        } catch(std::exception &e) {
+            return false;
+        }
+        return true;
+    }
+
+    unsigned int GetKeySize() {
+        return piter->key().size();
+    }
+
+    template<typename V> bool GetValue(V& value) {
+        leveldb::Slice slValue = piter->value();
+        try {
+            CDataStream ssValue(slValue.data(), slValue.data() + slValue.size(), SER_DISK, CLIENT_VERSION);
+            ssValue >> value;
+        } catch(std::exception &e) {
+            return false;
+        }
+        return true;
+    }
+
+    unsigned int GetValueSize() {
+        return piter->value().size();
+    }
+
+};
+
+class CDBWrapper
 {
 private:
     //! custom environment this database is using (may be NULL in case of default environment)
@@ -86,8 +156,14 @@ class CLevelDBWrapper
     leveldb::DB* pdb;
 
 public:
-    CLevelDBWrapper(const boost::filesystem::path& path, size_t nCacheSize, bool fMemory = false, bool fWipe = false);
-    ~CLevelDBWrapper();
+    /**
+     * @param[in] path        Location in the filesystem where leveldb data will be stored.
+     * @param[in] nCacheSize  Configures various leveldb cache settings.
+     * @param[in] fMemory     If true, use leveldb's memory environment.
+     * @param[in] fWipe       If true, remove all existing data.
+     */
+    CDBWrapper(const boost::filesystem::path& path, size_t nCacheSize, bool fMemory = false, bool fWipe = false);
+    ~CDBWrapper();
 
     template <typename K, typename V>
     bool Read(const K& key, V& value) const
@@ -103,7 +179,7 @@ class CLevelDBWrapper
             if (status.IsNotFound())
                 return false;
             LogPrintf("LevelDB read failure: %s\n", status.ToString());
-            HandleError(status);
+            dbwrapper_private::HandleError(status);
         }
         try {
             CDataStream ssValue(strValue.data(), strValue.data() + strValue.size(), SER_DISK, CLIENT_VERSION);
@@ -117,7 +193,7 @@ class CLevelDBWrapper
     template <typename K, typename V>
     bool Write(const K& key, const V& value, bool fSync = false)
     {
-        CLevelDBBatch batch;
+        CDBBatch batch;
         batch.Write(key, value);
         return WriteBatch(batch, fSync);
     }
@@ -136,20 +212,20 @@ class CLevelDBWrapper
             if (status.IsNotFound())
                 return false;
             LogPrintf("LevelDB read failure: %s\n", status.ToString());
-            HandleError(status);
+            dbwrapper_private::HandleError(status);
         }
         return true;
     }
 
     template <typename K>
     bool Erase(const K& key, bool fSync = false)
     {
-        CLevelDBBatch batch;
+        CDBBatch batch;
         batch.Erase(key);
         return WriteBatch(batch, fSync);
     }
 
-    bool WriteBatch(CLevelDBBatch& batch, bool fSync = false);
+    bool WriteBatch(CDBBatch& batch, bool fSync = false);
 
     // not available for LevelDB; provide for compatibility with BDB
     bool Flush()
@@ -159,15 +235,21 @@ class CLevelDBWrapper
 
     bool Sync()
     {
-        CLevelDBBatch batch;
+        CDBBatch batch;
         return WriteBatch(batch, true);
     }
 
     // not exactly clean encapsulation, but it's easiest for now
-    leveldb::Iterator* NewIterator()
+    CDBIterator* NewIterator()
     {
-        return pdb->NewIterator(iteroptions);
+        return new CDBIterator(pdb->NewIterator(iteroptions));
     }
+
+   /**
+    * Return true if the database managed by this class contains no entries.
+    */
+    bool IsEmpty();
+
 };
 
-#endif // BITCOIN_LEVELDBWRAPPER_H
+#endif // BITCOIN_DBWRAPPER_H

src/rpc/blockchain.cpp

@@ -600,8 +600,6 @@ UniValue gettxoutsetinfo(const UniValue& params, bool fHelp)
             "\nExamples:\n" +
             HelpExampleCli("gettxoutsetinfo", "") + HelpExampleRpc("gettxoutsetinfo", ""));
 
-    LOCK(cs_main);
-
     UniValue ret(UniValue::VOBJ);
 
     CCoinsStats stats;

src/sporkdb.cpp

@@ -5,7 +5,7 @@
 #include "sporkdb.h"
 #include "spork.h"
 
-CSporkDB::CSporkDB(size_t nCacheSize, bool fMemory, bool fWipe) : CLevelDBWrapper(GetDataDir() / "sporks", nCacheSize, fMemory, fWipe) {}
+CSporkDB::CSporkDB(size_t nCacheSize, bool fMemory, bool fWipe) : CDBWrapper(GetDataDir() / "sporks", nCacheSize, fMemory, fWipe) {}
 
 bool CSporkDB::WriteSpork(const SporkId nSporkId, const CSporkMessage& spork)
 {

src/sporkdb.h

@@ -6,10 +6,10 @@
 #define PIVX_CSPORKDB_H
 
 #include <boost/filesystem/path.hpp>
-#include "leveldbwrapper.h"
+#include "dbwrapper.h"
 #include "spork.h"
 
-class CSporkDB : public CLevelDBWrapper
+class CSporkDB : public CDBWrapper
 {
 public:
     CSporkDB(size_t nCacheSize, bool fMemory = false, bool fWipe = false);