Merge #8574: [Wallet] refactor CWallet/CWalletDB/CDB

7184e25 [Wallet] refactor CWallet/CWalletDB/CDB (Jonas Schnelli)

Tree-SHA512: a1993dcfc3505459613e8be3f6560ef32466fd7c649bff358f12af118e633aadd648a090f4af60743a827c9cb624e4ec63eb0202326da4779fc18249bb77da1e

src/wallet/db.cpp

@@ -18,6 +18,7 @@
 #endif
 
 #include <boost/filesystem.hpp>
+#include <boost/foreach.hpp>
 #include <boost/thread.hpp>
 #include <boost/version.hpp>
 
@@ -145,7 +146,7 @@ void CDBEnv::MakeMock()
     fMockDb = true;
 }
 
-CDBEnv::VerifyResult CDBEnv::Verify(const std::string& strFile, bool (*recoverFunc)(CDBEnv& dbenv, const std::string& strFile))
+CDBEnv::VerifyResult CDBEnv::Verify(const std::string& strFile, bool (*recoverFunc)(const std::string& strFile))
 {
     LOCK(cs_db);
     assert(mapFileUseCount.count(strFile) == 0);
@@ -158,10 +159,134 @@ CDBEnv::VerifyResult CDBEnv::Verify(const std::string& strFile, bool (*recoverFu
         return RECOVER_FAIL;
 
     // Try to recover:
-    bool fRecovered = (*recoverFunc)(*this, strFile);
+    bool fRecovered = (*recoverFunc)(strFile);
     return (fRecovered ? RECOVER_OK : RECOVER_FAIL);
 }
 
+bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue))
+{
+    // Recovery procedure:
+    // move wallet file to wallet.timestamp.bak
+    // Call Salvage with fAggressive=true to
+    // get as much data as possible.
+    // Rewrite salvaged data to fresh wallet file
+    // Set -rescan so any missing transactions will be
+    // found.
+    int64_t now = GetTime();
+    std::string newFilename = strprintf("wallet.%d.bak", now);
+
+    int result = bitdb.dbenv->dbrename(NULL, filename.c_str(), NULL,
+                                       newFilename.c_str(), DB_AUTO_COMMIT);
+    if (result == 0)
+        LogPrintf("Renamed %s to %s\n", filename, newFilename);
+    else
+    {
+        LogPrintf("Failed to rename %s to %s\n", filename, newFilename);
+        return false;
+    }
+
+    std::vector<CDBEnv::KeyValPair> salvagedData;
+    bool fSuccess = bitdb.Salvage(newFilename, true, salvagedData);
+    if (salvagedData.empty())
+    {
+        LogPrintf("Salvage(aggressive) found no records in %s.\n", newFilename);
+        return false;
+    }
+    LogPrintf("Salvage(aggressive) found %u records\n", salvagedData.size());
+
+    std::unique_ptr<Db> pdbCopy(new Db(bitdb.dbenv, 0));
+    int ret = pdbCopy->open(NULL,               // Txn pointer
+                            filename.c_str(),   // Filename
+                            "main",             // Logical db name
+                            DB_BTREE,           // Database type
+                            DB_CREATE,          // Flags
+                            0);
+    if (ret > 0)
+    {
+        LogPrintf("Cannot create database file %s\n", filename);
+        return false;
+    }
+
+    DbTxn* ptxn = bitdb.TxnBegin();
+    BOOST_FOREACH(CDBEnv::KeyValPair& row, salvagedData)
+    {
+        if (recoverKVcallback)
+        {
+            CDataStream ssKey(row.first, SER_DISK, CLIENT_VERSION);
+            CDataStream ssValue(row.second, SER_DISK, CLIENT_VERSION);
+            string strType, strErr;
+            if (!(*recoverKVcallback)(callbackDataIn, ssKey, ssValue))
+                continue;
+        }
+        Dbt datKey(&row.first[0], row.first.size());
+        Dbt datValue(&row.second[0], row.second.size());
+        int ret2 = pdbCopy->put(ptxn, &datKey, &datValue, DB_NOOVERWRITE);
+        if (ret2 > 0)
+            fSuccess = false;
+    }
+    ptxn->commit(0);
+    pdbCopy->close(0);
+
+    return fSuccess;
+}
+
+bool CDB::VerifyEnvironment(const std::string& walletFile, const boost::filesystem::path& dataDir, std::string& errorStr)
+{
+    LogPrintf("Using BerkeleyDB version %s\n", DbEnv::version(0, 0, 0));
+    LogPrintf("Using wallet %s\n", walletFile);
+
+    // Wallet file must be a plain filename without a directory
+    if (walletFile != boost::filesystem::basename(walletFile) + boost::filesystem::extension(walletFile))
+    {
+        errorStr = strprintf(_("Wallet %s resides outside data directory %s"), walletFile, dataDir.string());
+        return false;
+    }
+
+    if (!bitdb.Open(dataDir))
+    {
+        // try moving the database env out of the way
+        boost::filesystem::path pathDatabase = dataDir / "database";
+        boost::filesystem::path pathDatabaseBak = dataDir / strprintf("database.%d.bak", GetTime());
+        try {
+            boost::filesystem::rename(pathDatabase, pathDatabaseBak);
+            LogPrintf("Moved old %s to %s. Retrying.\n", pathDatabase.string(), pathDatabaseBak.string());
+        } catch (const boost::filesystem::filesystem_error&) {
+            // failure is ok (well, not really, but it's not worse than what we started with)
+        }
+
+        // try again
+        if (!bitdb.Open(dataDir)) {
+            // if it still fails, it probably means we can't even create the database env
+            errorStr = strprintf(_("Error initializing wallet database environment %s!"), GetDataDir());
+            return false;
+        }
+    }
+    return true;
+}
+
+bool CDB::VerifyDatabaseFile(const std::string& walletFile, const boost::filesystem::path& dataDir, std::string& warningStr, std::string& errorStr, bool (*recoverFunc)(const std::string& strFile))
+{
+    if (boost::filesystem::exists(dataDir / walletFile))
+    {
+        CDBEnv::VerifyResult r = bitdb.Verify(walletFile, recoverFunc);
+        if (r == CDBEnv::RECOVER_OK)
+        {
+            warningStr = strprintf(_("Warning: Wallet file corrupt, data salvaged!"
+                                     " Original %s saved as %s in %s; if"
+                                     " your balance or transactions are incorrect you should"
+                                     " restore from a backup."),
+                                   walletFile, "wallet.{timestamp}.bak", dataDir);
+        }
+        if (r == CDBEnv::RECOVER_FAIL)
+        {
+            errorStr = strprintf(_("%s corrupt, salvage failed"), walletFile);
+            return false;
+        }
+    }
+    // also return true if files does not exists
+    return true;
+}
+
 /* End of headers, beginning of key/value data */
 static const char *HEADER_END = "HEADER=END";
 /* End of key/value data */
@@ -473,3 +598,41 @@ void CDBEnv::Flush(bool fShutdown)
         }
     }
 }
+
+bool CDB::PeriodicFlush(std::string strFile)
+{
+    bool ret = false;
+    TRY_LOCK(bitdb.cs_db,lockDb);
+    if (lockDb)
+    {
+        // Don't do this if any databases are in use
+        int nRefCount = 0;
+        map<string, int>::iterator mi = bitdb.mapFileUseCount.begin();
+        while (mi != bitdb.mapFileUseCount.end())
+        {
+            nRefCount += (*mi).second;
+            mi++;
+        }
+
+        if (nRefCount == 0)
+        {
+            boost::this_thread::interruption_point();
+            map<string, int>::iterator mi = bitdb.mapFileUseCount.find(strFile);
+            if (mi != bitdb.mapFileUseCount.end())
+            {
+                LogPrint("db", "Flushing %s\n", strFile);
+                int64_t nStart = GetTimeMillis();
+
+                // Flush wallet file so it's self contained
+                bitdb.CloseDb(strFile);
+                bitdb.CheckpointLSN(strFile);
+
+                bitdb.mapFileUseCount.erase(mi++);
+                LogPrint("db", "Flushed %s %dms\n", strFile, GetTimeMillis() - nStart);
+                ret = true;
+            }
+        }
+    }
+
+    return ret;
+}

src/wallet/db.h

@@ -56,7 +56,7 @@ public:
     enum VerifyResult { VERIFY_OK,
                         RECOVER_OK,
                         RECOVER_FAIL };
-    VerifyResult Verify(const std::string& strFile, bool (*recoverFunc)(CDBEnv& dbenv, const std::string& strFile));
+    VerifyResult Verify(const std::string& strFile, bool (*recoverFunc)(const std::string& strFile));
     /**
      * Salvage data from a file that Verify says is bad.
      * fAggressive sets the DB_AGGRESSIVE flag (see berkeley DB->verify() method documentation).
@@ -104,6 +104,15 @@ protected:
 public:
     void Flush();
     void Close();
+    static bool Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue));
+
+    /* flush the wallet passively (TRY_LOCK)
+       ideal to be called periodically */
+    static bool PeriodicFlush(std::string strFile);
+    /* verifies the database environment */
+    static bool VerifyEnvironment(const std::string& walletFile, const boost::filesystem::path& dataDir, std::string& errorStr);
+    /* verifies the database file */
+    static bool VerifyDatabaseFile(const std::string& walletFile, const boost::filesystem::path& dataDir, std::string& warningStr, std::string& errorStr, bool (*recoverFunc)(const std::string& strFile));
 
 private:
     CDB(const CDB&);

src/wallet/wallet.cpp

@@ -444,57 +444,30 @@ bool CWallet::Verify()
     if (GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET))
         return true;
 
-    LogPrintf("Using BerkeleyDB version %s\n", DbEnv::version(0, 0, 0));
-    std::string walletFile = GetArg("-wallet", DEFAULT_WALLET_DAT);
-
-    LogPrintf("Using wallet %s\n", walletFile);
     uiInterface.InitMessage(_("Verifying wallet..."));
+    std::string walletFile = GetArg("-wallet", DEFAULT_WALLET_DAT);
 
-    // Wallet file must be a plain filename without a directory
+    std::string strError;
-    if (walletFile != boost::filesystem::basename(walletFile) + boost::filesystem::extension(walletFile))
+    if (!CWalletDB::VerifyEnvironment(walletFile, GetDataDir().string(), strError))
-        return InitError(strprintf(_("Wallet %s resides outside data directory %s"), walletFile, GetDataDir().string()));
+        return InitError(strError);
-
-    if (!bitdb.Open(GetDataDir()))
-    {
-        // try moving the database env out of the way
-        boost::filesystem::path pathDatabase = GetDataDir() / "database";
-        boost::filesystem::path pathDatabaseBak = GetDataDir() / strprintf("database.%d.bak", GetTime());
-        try {
-            boost::filesystem::rename(pathDatabase, pathDatabaseBak);
-            LogPrintf("Moved old %s to %s. Retrying.\n", pathDatabase.string(), pathDatabaseBak.string());
-        } catch (const boost::filesystem::filesystem_error&) {
-            // failure is ok (well, not really, but it's not worse than what we started with)
-        }
-        
-        // try again
-        if (!bitdb.Open(GetDataDir())) {
-            // if it still fails, it probably means we can't even create the database env
-            return InitError(strprintf(_("Error initializing wallet database environment %s!"), GetDataDir()));
-        }
-    }
 
     if (GetBoolArg("-salvagewallet", false))
     {
         // Recover readable keypairs:
-        if (!CWalletDB::Recover(bitdb, walletFile, true))
+        CWallet dummyWallet;
+        if (!CWalletDB::Recover(walletFile, (void *)&dummyWallet, CWalletDB::RecoverKeysOnlyFilter))
             return false;
     }
 
-    if (boost::filesystem::exists(GetDataDir() / walletFile))
+    std::string strWarning;
-    {
+    bool dbV = CWalletDB::VerifyDatabaseFile(walletFile, GetDataDir().string(), strWarning, strError);
-        CDBEnv::VerifyResult r = bitdb.Verify(walletFile, CWalletDB::Recover);
+    if (!strWarning.empty())
-        if (r == CDBEnv::RECOVER_OK)
+        InitWarning(strWarning);
+    if (!dbV)
     {
-            InitWarning(strprintf(_("Warning: Wallet file corrupt, data salvaged!"
+        InitError(strError);
-                                         " Original %s saved as %s in %s; if"
+        return false;
-                                         " your balance or transactions are incorrect you should"
-                                         " restore from a backup."),
-                walletFile, "wallet.{timestamp}.bak", GetDataDir()));
-        }
-        if (r == CDBEnv::RECOVER_FAIL)
-            return InitError(strprintf(_("%s corrupt, salvage failed"), walletFile));
     }
-    
     return true;
 }
 

src/wallet/walletdb.cpp

@@ -546,7 +546,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
     return true;
 }
 
-static bool IsKeyType(string strType)
+bool CWalletDB::IsKeyType(const std::string& strType)
 {
     return (strType== "key" || strType == "wkey" ||
             strType == "mkey" || strType == "ckey");
@@ -804,38 +804,9 @@ void ThreadFlushWalletDB()
 
         if (nLastFlushed != CWalletDB::GetUpdateCounter() && GetTime() - nLastWalletUpdate >= 2)
         {
-            TRY_LOCK(bitdb.cs_db,lockDb);
-            if (lockDb)
-            {
-                // Don't do this if any databases are in use
-                int nRefCount = 0;
-                map<string, int>::iterator mi = bitdb.mapFileUseCount.begin();
-                while (mi != bitdb.mapFileUseCount.end())
-                {
-                    nRefCount += (*mi).second;
-                    mi++;
-                }
-
-                if (nRefCount == 0)
-                {
-                    boost::this_thread::interruption_point();
             const std::string& strFile = pwalletMain->strWalletFile;
-                    map<string, int>::iterator _mi = bitdb.mapFileUseCount.find(strFile);
+            if (CDB::PeriodicFlush(strFile))
-                    if (_mi != bitdb.mapFileUseCount.end())
-                    {
-                        LogPrint("db", "Flushing %s\n", strFile);
                 nLastFlushed = CWalletDB::GetUpdateCounter();
-                        int64_t nStart = GetTimeMillis();
-
-                        // Flush wallet file so it's self contained
-                        bitdb.CloseDb(strFile);
-                        bitdb.CheckpointLSN(strFile);
-
-                        bitdb.mapFileUseCount.erase(_mi++);
-                        LogPrint("db", "Flushed %s %dms\n", strFile, GetTimeMillis() - nStart);
-                    }
-                }
-            }
         }
     }
 }
@@ -843,90 +814,49 @@ void ThreadFlushWalletDB()
 //
 // Try to (very carefully!) recover wallet file if there is a problem.
 //
-bool CWalletDB::Recover(CDBEnv& dbenv, const std::string& filename, bool fOnlyKeys)
+bool CWalletDB::Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue))
-{
+{
-    // Recovery procedure:
+    return CDB::Recover(filename, callbackDataIn, recoverKVcallback);
-    // move wallet file to wallet.timestamp.bak
+}
-    // Call Salvage with fAggressive=true to
-    // get as much data as possible.
-    // Rewrite salvaged data to fresh wallet file
-    // Set -rescan so any missing transactions will be
-    // found.
-    int64_t now = GetTime();
-    std::string newFilename = strprintf("wallet.%d.bak", now);
-
-    int result = dbenv.dbenv->dbrename(NULL, filename.c_str(), NULL,
-                                       newFilename.c_str(), DB_AUTO_COMMIT);
-    if (result == 0)
-        LogPrintf("Renamed %s to %s\n", filename, newFilename);
-    else
-    {
-        LogPrintf("Failed to rename %s to %s\n", filename, newFilename);
-        return false;
-    }
-
-    std::vector<CDBEnv::KeyValPair> salvagedData;
-    bool fSuccess = dbenv.Salvage(newFilename, true, salvagedData);
-    if (salvagedData.empty())
-    {
-        LogPrintf("Salvage(aggressive) found no records in %s.\n", newFilename);
-        return false;
-    }
-    LogPrintf("Salvage(aggressive) found %u records\n", salvagedData.size());
 
-    std::unique_ptr<Db> pdbCopy(new Db(dbenv.dbenv, 0));
+bool CWalletDB::Recover(const std::string& filename)
-    int ret = pdbCopy->open(NULL,               // Txn pointer
+{
-                            filename.c_str(),   // Filename
+    // recover without a key filter callback
-                            "main",             // Logical db name
+    // results in recovering all record types
-                            DB_BTREE,           // Database type
+    return CWalletDB::Recover(filename, NULL, NULL);
-                            DB_CREATE,          // Flags
+}
-                            0);
-    if (ret > 0)
-    {
-        LogPrintf("Cannot create database file %s\n", filename);
-        return false;
-    }
-    CWallet dummyWallet;
-    CWalletScanState wss;
 
-    DbTxn* ptxn = dbenv.TxnBegin();
+bool CWalletDB::RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue)
-    BOOST_FOREACH(CDBEnv::KeyValPair& row, salvagedData)
+{
-    {
+    CWallet *dummyWallet = reinterpret_cast<CWallet*>(callbackData);
-        if (fOnlyKeys)
+    CWalletScanState dummyWss;
-        {
+    std::string strType, strErr;
-            CDataStream ssKey(row.first, SER_DISK, CLIENT_VERSION);
-            CDataStream ssValue(row.second, SER_DISK, CLIENT_VERSION);
-            string strType, strErr;
     bool fReadOK;
     {
         // Required in LoadKeyMetadata():
-                LOCK(dummyWallet.cs_wallet);
+        LOCK(dummyWallet->cs_wallet);
-                fReadOK = ReadKeyValue(&dummyWallet, ssKey, ssValue,
+        fReadOK = ReadKeyValue(dummyWallet, ssKey, ssValue,
-                                        wss, strType, strErr);
+                               dummyWss, strType, strErr);
     }
     if (!IsKeyType(strType) && strType != "hdchain")
-                continue;
+        return false;
     if (!fReadOK)
     {
         LogPrintf("WARNING: CWalletDB::Recover skipping %s: %s\n", strType, strErr);
-                continue;
+        return false;
-            }
-        }
-        Dbt datKey(&row.first[0], row.first.size());
-        Dbt datValue(&row.second[0], row.second.size());
-        int ret2 = pdbCopy->put(ptxn, &datKey, &datValue, DB_NOOVERWRITE);
-        if (ret2 > 0)
-            fSuccess = false;
     }
-    ptxn->commit(0);
-    pdbCopy->close(0);
 
-    return fSuccess;
+    return true;
+}
+
+bool CWalletDB::VerifyEnvironment(const std::string& walletFile, const boost::filesystem::path& dataDir, std::string& errorStr)
+{
+    return CDB::VerifyEnvironment(walletFile, dataDir, errorStr);
 }
 
-bool CWalletDB::Recover(CDBEnv& dbenv, const std::string& filename)
+bool CWalletDB::VerifyDatabaseFile(const std::string& walletFile, const boost::filesystem::path& dataDir, std::string& warningStr, std::string& errorStr)
 {
-    return CWalletDB::Recover(dbenv, filename, false);
+    return CDB::VerifyDatabaseFile(walletFile, dataDir, errorStr, warningStr, CWalletDB::Recover);
 }
 
 bool CWalletDB::WriteDestData(const std::string &address, const std::string &key, const std::string &value)

src/wallet/walletdb.h

@@ -170,8 +170,18 @@ public:
     DBErrors FindWalletTx(std::vector<uint256>& vTxHash, std::vector<CWalletTx>& vWtx);
     DBErrors ZapWalletTx(std::vector<CWalletTx>& vWtx);
     DBErrors ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut);
-    static bool Recover(CDBEnv& dbenv, const std::string& filename, bool fOnlyKeys);
+    /* Try to (very carefully!) recover wallet database (with a possible key type filter) */
-    static bool Recover(CDBEnv& dbenv, const std::string& filename);
+    static bool Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue));
+    /* Recover convenience-function to bypass the key filter callback, called when verify failes, recoveres everything */
+    static bool Recover(const std::string& filename);
+    /* Recover filter (used as callback), will only let keys (cryptographical keys) as KV/key-type pass through */
+    static bool RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue);
+    /* Function to determin if a certain KV/key-type is a key (cryptographical key) type */
+    static bool IsKeyType(const std::string& strType);
+    /* verifies the database environment */
+    static bool VerifyEnvironment(const std::string& walletFile, const boost::filesystem::path& dataDir, std::string& errorStr);
+    /* verifies the database file */
+    static bool VerifyDatabaseFile(const std::string& walletFile, const boost::filesystem::path& dataDir, std::string& warningStr, std::string& errorStr);
 
     //! write the hdchain model (external chain child index counter)
     bool WriteHDChain(const CHDChain& chain);