From 71afe3c0995592ff17968816a833a8ed3ce05bf2 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 8 Mar 2017 11:48:58 +0100 Subject: [PATCH 1/6] wallet: Introduce database handle wrapper Abstract database handle from explicit strFilename into CWalletDBWrapper. Also move CWallet::Backup to db.cpp - as it deals with representation details this is a database specific operation. --- src/qt/test/wallettests.cpp | 3 +- src/wallet/db.cpp | 58 +++++++++-- src/wallet/db.h | 36 ++++++- src/wallet/rpcwallet.cpp | 2 +- src/wallet/test/wallet_test_fixture.cpp | 3 +- src/wallet/wallet.cpp | 129 ++++++++++-------------- src/wallet/wallet.h | 24 ++++- src/wallet/walletdb.cpp | 5 +- src/wallet/walletdb.h | 8 +- 9 files changed, 171 insertions(+), 97 deletions(-) diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index f794b6b382..a0dce3d997 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -86,7 +86,8 @@ void WalletTests::walletTests() TestChain100Setup test; test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey())); bitdb.MakeMock(); - CWallet wallet("wallet_test.dat"); + std::unique_ptr dbw(new CWalletDBWrapper(&bitdb, "wallet_test.dat")); + CWallet wallet(std::move(dbw)); bool firstRun; wallet.LoadWallet(firstRun); { diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index f47fc92b57..cdf785a957 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -359,13 +359,12 @@ void CDBEnv::CheckpointLSN(const std::string& strFile) } -CDB::CDB(const std::string& strFilename, const char* pszMode, bool fFlushOnCloseIn) : pdb(NULL), activeTxn(NULL) +CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb(NULL), activeTxn(NULL) { int ret; fReadOnly = (!strchr(pszMode, '+') && !strchr(pszMode, 'w')); fFlushOnClose = fFlushOnCloseIn; - if (strFilename.empty()) - return; + const std::string& strFilename = dbw.strFile; bool fCreate = strchr(pszMode, 'c') != NULL; unsigned int nFlags = DB_THREAD; @@ -472,8 +471,12 @@ bool CDBEnv::RemoveDb(const std::string& strFile) return (rc == 0); } -bool CDB::Rewrite(const std::string& strFile, const char* pszSkip) +bool CDB::Rewrite(CWalletDBWrapper& dbw, const char* pszSkip) { + if (!dbw.env) { + return true; + } + const std::string& strFile = dbw.strFile; while (true) { { LOCK(bitdb.cs_db); @@ -487,7 +490,7 @@ bool CDB::Rewrite(const std::string& strFile, const char* pszSkip) LogPrintf("CDB::Rewrite: Rewriting %s...\n", strFile); std::string strFileRes = strFile + ".rewrite"; { // surround usage of db with extra {} - CDB db(strFile.c_str(), "r"); + CDB db(dbw, "r"); Db* pdbCopy = new Db(bitdb.dbenv, 0); int ret = pdbCopy->open(NULL, // Txn pointer @@ -596,9 +599,10 @@ void CDBEnv::Flush(bool fShutdown) } } -bool CDB::PeriodicFlush(std::string strFile) +bool CDB::PeriodicFlush(CWalletDBWrapper& dbw) { bool ret = false; + const std::string& strFile = dbw.strFile; TRY_LOCK(bitdb.cs_db,lockDb); if (lockDb) { @@ -633,3 +637,45 @@ bool CDB::PeriodicFlush(std::string strFile) return ret; } + +bool CWalletDBWrapper::Rewrite(const char* pszSkip) +{ + return CDB::Rewrite(*this, pszSkip); +} + +bool CWalletDBWrapper::Backup(const std::string& strDest) +{ + if (!env) { + return false; + } + while (true) + { + { + LOCK(bitdb.cs_db); + if (!bitdb.mapFileUseCount.count(strFile) || bitdb.mapFileUseCount[strFile] == 0) + { + // Flush log data to the dat file + bitdb.CloseDb(strFile); + bitdb.CheckpointLSN(strFile); + bitdb.mapFileUseCount.erase(strFile); + + // Copy wallet file + fs::path pathSrc = GetDataDir() / strFile; + fs::path pathDest(strDest); + if (fs::is_directory(pathDest)) + pathDest /= strFile; + + try { + fs::copy_file(pathSrc, pathDest, fs::copy_option::overwrite_if_exists); + LogPrintf("copied %s to %s\n", strFile, pathDest.string()); + return true; + } catch (const fs::filesystem_error& e) { + LogPrintf("error copying %s to %s - %s\n", strFile, pathDest.string(), e.what()); + return false; + } + } + } + MilliSleep(100); + } + return false; +} diff --git a/src/wallet/db.h b/src/wallet/db.h index 9f912f9a1a..f6d0d54114 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -86,6 +86,36 @@ public: extern CDBEnv bitdb; +/** An instance of this class represents one database. + * For BerkeleyDB this is just a (env, strFile) tuple. + **/ +class CWalletDBWrapper +{ + friend class CDB; +public: + CWalletDBWrapper(CDBEnv *env_in, const std::string &strFile_in): + env(env_in), strFile(strFile_in) + { + } + + /** Rewrite the entire database on disk, with the exception of key pszSkip if non-zero + */ + bool Rewrite(const char* pszSkip=nullptr); + + /** Back up the entire database to a file. + */ + bool Backup(const std::string& strDest); + + /** Get a name for this database, for debugging etc. + */ + std::string GetName() const { return strFile; } + +private: + /** BerkeleyDB specific */ + CDBEnv *env; + std::string strFile; +}; + /** RAII class that provides access to a Berkeley database */ class CDB @@ -97,7 +127,7 @@ protected: bool fReadOnly; bool fFlushOnClose; - explicit CDB(const std::string& strFilename, const char* pszMode = "r+", bool fFlushOnCloseIn=true); + explicit CDB(CWalletDBWrapper& dbw, const char* pszMode = "r+", bool fFlushOnCloseIn=true); ~CDB() { Close(); } public: @@ -107,7 +137,7 @@ public: /* flush the wallet passively (TRY_LOCK) ideal to be called periodically */ - static bool PeriodicFlush(std::string strFile); + static bool PeriodicFlush(CWalletDBWrapper& dbw); /* verifies the database environment */ static bool VerifyEnvironment(const std::string& walletFile, const fs::path& dataDir, std::string& errorStr); /* verifies the database file */ @@ -310,7 +340,7 @@ public: return Write(std::string("version"), nVersion); } - bool static Rewrite(const std::string& strFile, const char* pszSkip = NULL); + bool static Rewrite(CWalletDBWrapper& dbw, const char* pszSkip = NULL); }; #endif // BITCOIN_WALLET_DB_H diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 665d856df5..7408806b7d 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2076,7 +2076,7 @@ UniValue walletpassphrase(const JSONRPCRequest& request) int64_t nSleepTime = request.params[1].get_int64(); pwallet->nRelockTime = GetTime() + nSleepTime; - RPCRunLater(strprintf("lockwallet(%s)", pwallet->strWalletFile), boost::bind(LockWallet, pwallet), nSleepTime); + RPCRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), boost::bind(LockWallet, pwallet), nSleepTime); return NullUniValue; } diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index a76db37617..1989bf8d9b 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -14,7 +14,8 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName): bitdb.MakeMock(); bool fFirstRun; - pwalletMain = new CWallet("wallet_test.dat"); + std::unique_ptr dbw(new CWalletDBWrapper(&bitdb, "wallet_test.dat")); + pwalletMain = new CWallet(std::move(dbw)); pwalletMain->LoadWallet(fFirstRun); RegisterValidationInterface(pwalletMain); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b5e26e26d3..5b03b00179 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -161,7 +161,7 @@ void CWallet::DeriveNewChildKey(CKeyMetadata& metadata, CKey& secret, bool inter secret = childKey.key; metadata.hdMasterKeyID = hdChain.masterKeyID; // update the chain model in the database - if (!CWalletDB(strWalletFile).WriteHDChain(hdChain)) + if (!CWalletDB(*dbw).WriteHDChain(hdChain)) throw std::runtime_error(std::string(__func__) + ": Writing HD chain model failed"); } @@ -183,7 +183,7 @@ bool CWallet::AddKeyPubKey(const CKey& secret, const CPubKey &pubkey) if (!fFileBacked) return true; if (!IsCrypted()) { - return CWalletDB(strWalletFile).WriteKey(pubkey, + return CWalletDB(*dbw).WriteKey(pubkey, secret.GetPrivKey(), mapKeyMetadata[pubkey.GetID()]); } @@ -204,7 +204,7 @@ bool CWallet::AddCryptedKey(const CPubKey &vchPubKey, vchCryptedSecret, mapKeyMetadata[vchPubKey.GetID()]); else - return CWalletDB(strWalletFile).WriteCryptedKey(vchPubKey, + return CWalletDB(*dbw).WriteCryptedKey(vchPubKey, vchCryptedSecret, mapKeyMetadata[vchPubKey.GetID()]); } @@ -242,7 +242,7 @@ bool CWallet::AddCScript(const CScript& redeemScript) return false; if (!fFileBacked) return true; - return CWalletDB(strWalletFile).WriteCScript(Hash160(redeemScript), redeemScript); + return CWalletDB(*dbw).WriteCScript(Hash160(redeemScript), redeemScript); } bool CWallet::LoadCScript(const CScript& redeemScript) @@ -270,7 +270,7 @@ bool CWallet::AddWatchOnly(const CScript& dest) NotifyWatchonlyChanged(true); if (!fFileBacked) return true; - return CWalletDB(strWalletFile).WriteWatchOnly(dest, meta); + return CWalletDB(*dbw).WriteWatchOnly(dest, meta); } bool CWallet::AddWatchOnly(const CScript& dest, int64_t nCreateTime) @@ -287,7 +287,7 @@ bool CWallet::RemoveWatchOnly(const CScript &dest) if (!HaveWatchOnly()) NotifyWatchonlyChanged(false); if (fFileBacked) - if (!CWalletDB(strWalletFile).EraseWatchOnly(dest)) + if (!CWalletDB(*dbw).EraseWatchOnly(dest)) return false; return true; @@ -353,7 +353,7 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, return false; if (!crypter.Encrypt(_vMasterKey, pMasterKey.second.vchCryptedKey)) return false; - CWalletDB(strWalletFile).WriteMasterKey(pMasterKey.first, pMasterKey.second); + CWalletDB(*dbw).WriteMasterKey(pMasterKey.first, pMasterKey.second); if (fWasLocked) Lock(); return true; @@ -366,7 +366,7 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, void CWallet::SetBestChain(const CBlockLocator& loc) { - CWalletDB walletdb(strWalletFile); + CWalletDB walletdb(*dbw); walletdb.WriteBestBlock(loc); } @@ -387,7 +387,7 @@ bool CWallet::SetMinVersion(enum WalletFeature nVersion, CWalletDB* pwalletdbIn, if (fFileBacked) { - CWalletDB* pwalletdb = pwalletdbIn ? pwalletdbIn : new CWalletDB(strWalletFile); + CWalletDB* pwalletdb = pwalletdbIn ? pwalletdbIn : new CWalletDB(*dbw); if (nWalletVersion > 40000) pwalletdb->WriteMinVersion(nWalletVersion); if (!pwalletdbIn) @@ -597,7 +597,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) if (fFileBacked) { assert(!pwalletdbEncryption); - pwalletdbEncryption = new CWalletDB(strWalletFile); + pwalletdbEncryption = new CWalletDB(*dbw); if (!pwalletdbEncryption->TxnBegin()) { delete pwalletdbEncryption; pwalletdbEncryption = NULL; @@ -651,7 +651,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) // Need to completely rewrite the wallet file; if we don't, bdb might keep // bits of the unencrypted private key in slack space in the database file. - CDB::Rewrite(strWalletFile); + dbw->Rewrite(); } NotifyStatusChanged(this); @@ -662,7 +662,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) DBErrors CWallet::ReorderTransactions() { LOCK(cs_wallet); - CWalletDB walletdb(strWalletFile); + CWalletDB walletdb(*dbw); // Old wallets didn't have any defined order for transactions // Probably a bad idea to change the output of this @@ -743,14 +743,14 @@ int64_t CWallet::IncOrderPosNext(CWalletDB *pwalletdb) if (pwalletdb) { pwalletdb->WriteOrderPosNext(nOrderPosNext); } else { - CWalletDB(strWalletFile).WriteOrderPosNext(nOrderPosNext); + CWalletDB(*dbw).WriteOrderPosNext(nOrderPosNext); } return nRet; } bool CWallet::AccountMove(std::string strFrom, std::string strTo, CAmount nAmount, std::string strComment) { - CWalletDB walletdb(strWalletFile); + CWalletDB walletdb(*dbw); if (!walletdb.TxnBegin()) return false; @@ -784,7 +784,7 @@ bool CWallet::AccountMove(std::string strFrom, std::string strTo, CAmount nAmoun bool CWallet::GetAccountPubkey(CPubKey &pubKey, std::string strAccount, bool bForceNew) { - CWalletDB walletdb(strWalletFile); + CWalletDB walletdb(*dbw); CAccount account; walletdb.ReadAccount(strAccount, account); @@ -845,7 +845,7 @@ bool CWallet::MarkReplaced(const uint256& originalHash, const uint256& newHash) wtx.mapValue["replaced_by_txid"] = newHash.ToString(); - CWalletDB walletdb(strWalletFile, "r+"); + CWalletDB walletdb(*dbw, "r+"); bool success = true; if (!walletdb.WriteTx(wtx)) { @@ -862,7 +862,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose) { LOCK(cs_wallet); - CWalletDB walletdb(strWalletFile, "r+", fFlushOnClose); + CWalletDB walletdb(*dbw, "r+", fFlushOnClose); uint256 hash = wtxIn.GetHash(); @@ -1006,7 +1006,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) { LOCK2(cs_main, cs_wallet); - CWalletDB walletdb(strWalletFile, "r+"); + CWalletDB walletdb(*dbw, "r+"); std::set todo; std::set done; @@ -1078,7 +1078,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) return; // Do not flush the wallet here for performance reasons - CWalletDB walletdb(strWalletFile, "r+", false); + CWalletDB walletdb(*dbw, "r+", false); std::set todo; std::set done; @@ -1361,7 +1361,7 @@ bool CWallet::SetHDMasterKey(const CPubKey& pubkey, CHDChain *possibleOldChain) bool CWallet::SetHDChain(const CHDChain& chain, bool memonly) { LOCK(cs_wallet); - if (!memonly && !CWalletDB(strWalletFile).WriteHDChain(chain)) + if (!memonly && !CWalletDB(*dbw).WriteHDChain(chain)) throw std::runtime_error(std::string(__func__) + ": writing chain failed"); hdChain = chain; @@ -2758,13 +2758,13 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CCon } void CWallet::ListAccountCreditDebit(const std::string& strAccount, std::list& entries) { - CWalletDB walletdb(strWalletFile); + CWalletDB walletdb(*dbw); return walletdb.ListAccountCreditDebit(strAccount, entries); } bool CWallet::AddAccountingEntry(const CAccountingEntry& acentry) { - CWalletDB walletdb(strWalletFile); + CWalletDB walletdb(*dbw); return AddAccountingEntry(acentry, &walletdb); } @@ -2819,10 +2819,10 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) if (!fFileBacked) return DB_LOAD_OK; fFirstRunRet = false; - DBErrors nLoadWalletRet = CWalletDB(strWalletFile,"cr+").LoadWallet(this); + DBErrors nLoadWalletRet = CWalletDB(*dbw,"cr+").LoadWallet(this); if (nLoadWalletRet == DB_NEED_REWRITE) { - if (CDB::Rewrite(strWalletFile, "\x04pool")) + if (dbw->Rewrite("\x04pool")) { LOCK(cs_wallet); setKeyPool.clear(); @@ -2847,13 +2847,13 @@ DBErrors CWallet::ZapSelectTx(std::vector& vHashIn, std::vectorRewrite("\x04pool")) { setKeyPool.clear(); // Note: can't top-up keypool here, because wallet is locked. @@ -2876,10 +2876,10 @@ DBErrors CWallet::ZapWalletTx(std::vector& vWtx) if (!fFileBacked) return DB_LOAD_OK; vchDefaultKey = CPubKey(); - DBErrors nZapWalletTxRet = CWalletDB(strWalletFile,"cr+").ZapWalletTx(vWtx); + DBErrors nZapWalletTxRet = CWalletDB(*dbw,"cr+").ZapWalletTx(vWtx); if (nZapWalletTxRet == DB_NEED_REWRITE) { - if (CDB::Rewrite(strWalletFile, "\x04pool")) + if (dbw->Rewrite("\x04pool")) { LOCK(cs_wallet); setKeyPool.clear(); @@ -2911,9 +2911,9 @@ bool CWallet::SetAddressBook(const CTxDestination& address, const std::string& s strPurpose, (fUpdated ? CT_UPDATED : CT_NEW) ); if (!fFileBacked) return false; - if (!strPurpose.empty() && !CWalletDB(strWalletFile).WritePurpose(CBitcoinAddress(address).ToString(), strPurpose)) + if (!strPurpose.empty() && !CWalletDB(*dbw).WritePurpose(CBitcoinAddress(address).ToString(), strPurpose)) return false; - return CWalletDB(strWalletFile).WriteName(CBitcoinAddress(address).ToString(), strName); + return CWalletDB(*dbw).WriteName(CBitcoinAddress(address).ToString(), strName); } bool CWallet::DelAddressBook(const CTxDestination& address) @@ -2927,7 +2927,7 @@ bool CWallet::DelAddressBook(const CTxDestination& address) std::string strAddress = CBitcoinAddress(address).ToString(); BOOST_FOREACH(const PAIRTYPE(std::string, std::string) &item, mapAddressBook[address].destdata) { - CWalletDB(strWalletFile).EraseDestData(strAddress, item.first); + CWalletDB(*dbw).EraseDestData(strAddress, item.first); } } mapAddressBook.erase(address); @@ -2937,15 +2937,15 @@ bool CWallet::DelAddressBook(const CTxDestination& address) if (!fFileBacked) return false; - CWalletDB(strWalletFile).ErasePurpose(CBitcoinAddress(address).ToString()); - return CWalletDB(strWalletFile).EraseName(CBitcoinAddress(address).ToString()); + CWalletDB(*dbw).ErasePurpose(CBitcoinAddress(address).ToString()); + return CWalletDB(*dbw).EraseName(CBitcoinAddress(address).ToString()); } bool CWallet::SetDefaultKey(const CPubKey &vchPubKey) { if (fFileBacked) { - if (!CWalletDB(strWalletFile).WriteDefaultKey(vchPubKey)) + if (!CWalletDB(*dbw).WriteDefaultKey(vchPubKey)) return false; } vchDefaultKey = vchPubKey; @@ -2960,7 +2960,7 @@ bool CWallet::NewKeyPool() { { LOCK(cs_wallet); - CWalletDB walletdb(strWalletFile); + CWalletDB walletdb(*dbw); BOOST_FOREACH(int64_t nIndex, setKeyPool) walletdb.ErasePool(nIndex); setKeyPool.clear(); @@ -2981,7 +2981,7 @@ size_t CWallet::KeypoolCountExternalKeys() if (!IsHDEnabled() || !CanSupportFeature(FEATURE_HD_SPLIT)) return setKeyPool.size(); - CWalletDB walletdb(strWalletFile); + CWalletDB walletdb(*dbw); // count amount of external keys size_t amountE = 0; @@ -3024,7 +3024,7 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize) missingInternal = 0; } bool internal = false; - CWalletDB walletdb(strWalletFile); + CWalletDB walletdb(*dbw); for (int64_t i = missingInternal + missingExternal; i--;) { int64_t nEnd = 1; @@ -3055,7 +3055,7 @@ void CWallet::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool int if(setKeyPool.empty()) return; - CWalletDB walletdb(strWalletFile); + CWalletDB walletdb(*dbw); // try to find a key that matches the internal/external filter for(const int64_t& id : setKeyPool) @@ -3083,7 +3083,7 @@ void CWallet::KeepKey(int64_t nIndex) // Remove from key pool if (fFileBacked) { - CWalletDB walletdb(strWalletFile); + CWalletDB walletdb(*dbw); walletdb.ErasePool(nIndex); } LogPrintf("keypool keep %d\n", nIndex); @@ -3127,7 +3127,7 @@ int64_t CWallet::GetOldestKeyPoolTime() return GetTime(); CKeyPool keypool; - CWalletDB walletdb(strWalletFile); + CWalletDB walletdb(*dbw); if (IsHDEnabled() && CanSupportFeature(FEATURE_HD_SPLIT)) { @@ -3295,7 +3295,7 @@ std::set< std::set > CWallet::GetAddressGroupings() CAmount CWallet::GetAccountBalance(const std::string& strAccount, int nMinDepth, const isminefilter& filter) { - CWalletDB walletdb(strWalletFile); + CWalletDB walletdb(*dbw); return GetAccountBalance(walletdb, strAccount, nMinDepth, filter); } @@ -3375,7 +3375,7 @@ void CWallet::GetAllReserveKeys(std::set& setAddress) const { setAddress.clear(); - CWalletDB walletdb(strWalletFile); + CWalletDB walletdb(*dbw); LOCK2(cs_main, cs_wallet); BOOST_FOREACH(const int64_t& id, setKeyPool) @@ -3599,7 +3599,7 @@ bool CWallet::AddDestData(const CTxDestination &dest, const std::string &key, co mapAddressBook[dest].destdata.insert(std::make_pair(key, value)); if (!fFileBacked) return true; - return CWalletDB(strWalletFile).WriteDestData(CBitcoinAddress(dest).ToString(), key, value); + return CWalletDB(*dbw).WriteDestData(CBitcoinAddress(dest).ToString(), key, value); } bool CWallet::EraseDestData(const CTxDestination &dest, const std::string &key) @@ -3608,7 +3608,7 @@ bool CWallet::EraseDestData(const CTxDestination &dest, const std::string &key) return false; if (!fFileBacked) return true; - return CWalletDB(strWalletFile).EraseDestData(CBitcoinAddress(dest).ToString(), key); + return CWalletDB(*dbw).EraseDestData(CBitcoinAddress(dest).ToString(), key); } bool CWallet::LoadDestData(const CTxDestination &dest, const std::string &key, const std::string &value) @@ -3678,7 +3678,8 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) if (GetBoolArg("-zapwallettxes", false)) { uiInterface.InitMessage(_("Zapping all transactions from wallet...")); - CWallet *tempWallet = new CWallet(walletFile); + std::unique_ptr dbw(new CWalletDBWrapper(&bitdb, walletFile)); + CWallet *tempWallet = new CWallet(std::move(dbw)); DBErrors nZapWalletRet = tempWallet->ZapWalletTx(vWtx); if (nZapWalletRet != DB_LOAD_OK) { InitError(strprintf(_("Error loading %s: Wallet corrupted"), walletFile)); @@ -3693,7 +3694,8 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) int64_t nStart = GetTimeMillis(); bool fFirstRun = true; - CWallet *walletInstance = new CWallet(walletFile); + std::unique_ptr dbw(new CWalletDBWrapper(&bitdb, walletFile)); + CWallet *walletInstance = new CWallet(std::move(dbw)); DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); if (nLoadWalletRet != DB_LOAD_OK) { @@ -3784,7 +3786,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) CBlockIndex *pindexRescan = chainActive.Genesis(); if (!GetBoolArg("-rescan", false)) { - CWalletDB walletdb(walletFile); + CWalletDB walletdb(*walletInstance->dbw); CBlockLocator locator; if (walletdb.ReadBestBlock(locator)) pindexRescan = FindForkInGlobalIndex(chainActive, locator); @@ -3817,7 +3819,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) // Restore wallet transaction metadata after -zapwallettxes=1 if (GetBoolArg("-zapwallettxes", false) && GetArg("-zapwallettxes", "1") != "2") { - CWalletDB walletdb(walletFile); + CWalletDB walletdb(*walletInstance->dbw); BOOST_FOREACH(const CWalletTx& wtxOld, vWtx) { @@ -3979,36 +3981,7 @@ bool CWallet::BackupWallet(const std::string& strDest) { if (!fFileBacked) return false; - while (true) - { - { - LOCK(bitdb.cs_db); - if (!bitdb.mapFileUseCount.count(strWalletFile) || bitdb.mapFileUseCount[strWalletFile] == 0) - { - // Flush log data to the dat file - bitdb.CloseDb(strWalletFile); - bitdb.CheckpointLSN(strWalletFile); - bitdb.mapFileUseCount.erase(strWalletFile); - - // Copy wallet file - fs::path pathSrc = GetDataDir() / strWalletFile; - fs::path pathDest(strDest); - if (fs::is_directory(pathDest)) - pathDest /= strWalletFile; - - try { - fs::copy_file(pathSrc, pathDest, fs::copy_option::overwrite_if_exists); - LogPrintf("copied %s to %s\n", strWalletFile, pathDest.string()); - return true; - } catch (const fs::filesystem_error& e) { - LogPrintf("error copying %s to %s - %s\n", strWalletFile, pathDest.string(), e.what()); - return false; - } - } - } - MilliSleep(100); - } - return false; + return dbw->Backup(strDest); } CKeyPool::CKeyPool() diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 06e7e14990..318aa2bf97 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -715,17 +715,35 @@ private: */ bool AddWatchOnly(const CScript& dest) override; + std::unique_ptr dbw; + public: /* * Main wallet lock. * This lock protects all the fields added by CWallet * except for: * fFileBacked (immutable after instantiation) - * strWalletFile (immutable after instantiation) */ mutable CCriticalSection cs_wallet; - const std::string strWalletFile; + /** Get database handle used by this wallet. Ideally this function would + * not be necessary. + */ + CWalletDBWrapper& GetDBHandle() + { + return *dbw; + } + + /** Get a name for this wallet for logging/debugging purposes. + */ + std::string GetName() const + { + if (dbw) { + return dbw->GetName(); + } else { + return "dummy"; + } + } void LoadKeyPool(int nIndex, const CKeyPool &keypool) { @@ -752,7 +770,7 @@ public: SetNull(); } - CWallet(const std::string& strWalletFileIn) : strWalletFile(strWalletFileIn) + CWallet(std::unique_ptr dbw_in) : dbw(std::move(dbw_in)) { SetNull(); fFileBacked = true; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index ceff2d36e3..6921c9900b 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -797,9 +797,9 @@ void MaybeCompactWalletDB() if (nLastFlushed != CWalletDB::GetUpdateCounter() && GetTime() - nLastWalletUpdate >= 2) { - const std::string& strFile = pwalletMain->strWalletFile; - if (CDB::PeriodicFlush(strFile)) + if (CDB::PeriodicFlush(pwalletMain->GetDBHandle())) { nLastFlushed = CWalletDB::GetUpdateCounter(); + } } fOneThread = false; } @@ -880,3 +880,4 @@ unsigned int CWalletDB::GetUpdateCounter() { return nWalletDBUpdateCounter; } + diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index b94f341b2e..c0135a6292 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -118,11 +118,15 @@ public: } }; -/** Access to the wallet database */ +/** Access to the wallet database. + * This should really be named CWalletDBBatch, as it represents a single transaction at the + * database. It will be committed when the object goes out of scope. + * Optionally (on by default) it will flush to disk as well. + */ class CWalletDB : public CDB { public: - CWalletDB(const std::string& strFilename, const char* pszMode = "r+", bool _fFlushOnClose = true) : CDB(strFilename, pszMode, _fFlushOnClose) + CWalletDB(CWalletDBWrapper& dbw, const char* pszMode = "r+", bool _fFlushOnClose = true) : CDB(dbw, pszMode, _fFlushOnClose) { } From 071c95570b8e2ee5c2a4aed8b86b9b14f8e75578 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 8 Mar 2017 12:08:26 +0000 Subject: [PATCH 2/6] wallet: Get rid of fFileBacked Instead, CWalletDB() with a dummy handle will just give you a no-op database in which writes always succeeds and reads always fail. CDB already had functionality for this, so just use that. --- src/wallet/db.cpp | 12 ++++-- src/wallet/db.h | 14 ++++++- src/wallet/wallet.cpp | 95 ++++++++++++------------------------------- src/wallet/wallet.h | 12 ++---- 4 files changed, 52 insertions(+), 81 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index cdf785a957..0ea4ea3673 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -364,7 +364,10 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb int ret; fReadOnly = (!strchr(pszMode, '+') && !strchr(pszMode, 'w')); fFlushOnClose = fFlushOnCloseIn; - const std::string& strFilename = dbw.strFile; + if (dbw.IsDummy()) { + return; + } + const std::string &strFilename = dbw.strFile; bool fCreate = strchr(pszMode, 'c') != NULL; unsigned int nFlags = DB_THREAD; @@ -473,7 +476,7 @@ bool CDBEnv::RemoveDb(const std::string& strFile) bool CDB::Rewrite(CWalletDBWrapper& dbw, const char* pszSkip) { - if (!dbw.env) { + if (dbw.IsDummy()) { return true; } const std::string& strFile = dbw.strFile; @@ -601,6 +604,9 @@ void CDBEnv::Flush(bool fShutdown) bool CDB::PeriodicFlush(CWalletDBWrapper& dbw) { + if (dbw.IsDummy()) { + return true; + } bool ret = false; const std::string& strFile = dbw.strFile; TRY_LOCK(bitdb.cs_db,lockDb); @@ -645,7 +651,7 @@ bool CWalletDBWrapper::Rewrite(const char* pszSkip) bool CWalletDBWrapper::Backup(const std::string& strDest) { - if (!env) { + if (IsDummy()) { return false; } while (true) diff --git a/src/wallet/db.h b/src/wallet/db.h index f6d0d54114..892c8d33c3 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -93,6 +93,12 @@ class CWalletDBWrapper { friend class CDB; public: + /** Create dummy DB handle */ + CWalletDBWrapper(): env(nullptr) + { + } + + /** Create DB handle to real database */ CWalletDBWrapper(CDBEnv *env_in, const std::string &strFile_in): env(env_in), strFile(strFile_in) { @@ -110,6 +116,12 @@ public: */ std::string GetName() const { return strFile; } + /** Return whether this database handle is a dummy for testing. + * Only to be used at a low level, application should ideally not care + * about this. + */ + bool IsDummy() { return env == nullptr; } + private: /** BerkeleyDB specific */ CDBEnv *env; @@ -186,7 +198,7 @@ protected: bool Write(const K& key, const T& value, bool fOverwrite = true) { if (!pdb) - return false; + return true; if (fReadOnly) assert(!"Write called on database in read-only mode"); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 5b03b00179..a9d5d02c07 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -180,8 +180,6 @@ bool CWallet::AddKeyPubKey(const CKey& secret, const CPubKey &pubkey) if (HaveWatchOnly(script)) RemoveWatchOnly(script); - if (!fFileBacked) - return true; if (!IsCrypted()) { return CWalletDB(*dbw).WriteKey(pubkey, secret.GetPrivKey(), @@ -195,8 +193,6 @@ bool CWallet::AddCryptedKey(const CPubKey &vchPubKey, { if (!CCryptoKeyStore::AddCryptedKey(vchPubKey, vchCryptedSecret)) return false; - if (!fFileBacked) - return true; { LOCK(cs_wallet); if (pwalletdbEncryption) @@ -240,8 +236,6 @@ bool CWallet::AddCScript(const CScript& redeemScript) { if (!CCryptoKeyStore::AddCScript(redeemScript)) return false; - if (!fFileBacked) - return true; return CWalletDB(*dbw).WriteCScript(Hash160(redeemScript), redeemScript); } @@ -268,8 +262,6 @@ bool CWallet::AddWatchOnly(const CScript& dest) const CKeyMetadata& meta = mapKeyMetadata[CScriptID(dest)]; UpdateTimeFirstKey(meta.nCreateTime); NotifyWatchonlyChanged(true); - if (!fFileBacked) - return true; return CWalletDB(*dbw).WriteWatchOnly(dest, meta); } @@ -286,9 +278,8 @@ bool CWallet::RemoveWatchOnly(const CScript &dest) return false; if (!HaveWatchOnly()) NotifyWatchonlyChanged(false); - if (fFileBacked) - if (!CWalletDB(*dbw).EraseWatchOnly(dest)) - return false; + if (!CWalletDB(*dbw).EraseWatchOnly(dest)) + return false; return true; } @@ -385,7 +376,6 @@ bool CWallet::SetMinVersion(enum WalletFeature nVersion, CWalletDB* pwalletdbIn, if (nVersion > nWalletMaxVersion) nWalletMaxVersion = nVersion; - if (fFileBacked) { CWalletDB* pwalletdb = pwalletdbIn ? pwalletdbIn : new CWalletDB(*dbw); if (nWalletVersion > 40000) @@ -594,24 +584,19 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) { LOCK(cs_wallet); mapMasterKeys[++nMasterKeyMaxID] = kMasterKey; - if (fFileBacked) - { - assert(!pwalletdbEncryption); - pwalletdbEncryption = new CWalletDB(*dbw); - if (!pwalletdbEncryption->TxnBegin()) { - delete pwalletdbEncryption; - pwalletdbEncryption = NULL; - return false; - } - pwalletdbEncryption->WriteMasterKey(nMasterKeyMaxID, kMasterKey); + assert(!pwalletdbEncryption); + pwalletdbEncryption = new CWalletDB(*dbw); + if (!pwalletdbEncryption->TxnBegin()) { + delete pwalletdbEncryption; + pwalletdbEncryption = NULL; + return false; } + pwalletdbEncryption->WriteMasterKey(nMasterKeyMaxID, kMasterKey); if (!EncryptKeys(_vMasterKey)) { - if (fFileBacked) { - pwalletdbEncryption->TxnAbort(); - delete pwalletdbEncryption; - } + pwalletdbEncryption->TxnAbort(); + delete pwalletdbEncryption; // We now probably have half of our keys encrypted in memory, and half not... // die and let the user reload the unencrypted wallet. assert(false); @@ -620,19 +605,16 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) // Encryption was introduced in version 0.4.0 SetMinVersion(FEATURE_WALLETCRYPT, pwalletdbEncryption, true); - if (fFileBacked) - { - if (!pwalletdbEncryption->TxnCommit()) { - delete pwalletdbEncryption; - // We now have keys encrypted in memory, but not on disk... - // die to avoid confusion and let the user reload the unencrypted wallet. - assert(false); - } - + if (!pwalletdbEncryption->TxnCommit()) { delete pwalletdbEncryption; - pwalletdbEncryption = NULL; + // We now have keys encrypted in memory, but not on disk... + // die to avoid confusion and let the user reload the unencrypted wallet. + assert(false); } + delete pwalletdbEncryption; + pwalletdbEncryption = NULL; + Lock(); Unlock(strWalletPassphrase); @@ -2816,8 +2798,6 @@ CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarge DBErrors CWallet::LoadWallet(bool& fFirstRunRet) { - if (!fFileBacked) - return DB_LOAD_OK; fFirstRunRet = false; DBErrors nLoadWalletRet = CWalletDB(*dbw,"cr+").LoadWallet(this); if (nLoadWalletRet == DB_NEED_REWRITE) @@ -2843,8 +2823,6 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) DBErrors CWallet::ZapSelectTx(std::vector& vHashIn, std::vector& vHashOut) { - if (!fFileBacked) - return DB_LOAD_OK; AssertLockHeld(cs_wallet); // mapWallet vchDefaultKey = CPubKey(); DBErrors nZapSelectTxRet = CWalletDB(*dbw,"cr+").ZapSelectTx(vHashIn, vHashOut); @@ -2873,8 +2851,6 @@ DBErrors CWallet::ZapSelectTx(std::vector& vHashIn, std::vector& vWtx) { - if (!fFileBacked) - return DB_LOAD_OK; vchDefaultKey = CPubKey(); DBErrors nZapWalletTxRet = CWalletDB(*dbw,"cr+").ZapWalletTx(vWtx); if (nZapWalletTxRet == DB_NEED_REWRITE) @@ -2909,8 +2885,6 @@ bool CWallet::SetAddressBook(const CTxDestination& address, const std::string& s } NotifyAddressBookChanged(this, address, strName, ::IsMine(*this, address) != ISMINE_NO, strPurpose, (fUpdated ? CT_UPDATED : CT_NEW) ); - if (!fFileBacked) - return false; if (!strPurpose.empty() && !CWalletDB(*dbw).WritePurpose(CBitcoinAddress(address).ToString(), strPurpose)) return false; return CWalletDB(*dbw).WriteName(CBitcoinAddress(address).ToString(), strName); @@ -2921,33 +2895,25 @@ bool CWallet::DelAddressBook(const CTxDestination& address) { LOCK(cs_wallet); // mapAddressBook - if(fFileBacked) + // Delete destdata tuples associated with address + std::string strAddress = CBitcoinAddress(address).ToString(); + BOOST_FOREACH(const PAIRTYPE(std::string, std::string) &item, mapAddressBook[address].destdata) { - // Delete destdata tuples associated with address - std::string strAddress = CBitcoinAddress(address).ToString(); - BOOST_FOREACH(const PAIRTYPE(std::string, std::string) &item, mapAddressBook[address].destdata) - { - CWalletDB(*dbw).EraseDestData(strAddress, item.first); - } + CWalletDB(*dbw).EraseDestData(strAddress, item.first); } mapAddressBook.erase(address); } NotifyAddressBookChanged(this, address, "", ::IsMine(*this, address) != ISMINE_NO, "", CT_DELETED); - if (!fFileBacked) - return false; CWalletDB(*dbw).ErasePurpose(CBitcoinAddress(address).ToString()); return CWalletDB(*dbw).EraseName(CBitcoinAddress(address).ToString()); } bool CWallet::SetDefaultKey(const CPubKey &vchPubKey) { - if (fFileBacked) - { - if (!CWalletDB(*dbw).WriteDefaultKey(vchPubKey)) - return false; - } + if (!CWalletDB(*dbw).WriteDefaultKey(vchPubKey)) + return false; vchDefaultKey = vchPubKey; return true; } @@ -3081,11 +3047,8 @@ void CWallet::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool int void CWallet::KeepKey(int64_t nIndex) { // Remove from key pool - if (fFileBacked) - { - CWalletDB walletdb(*dbw); - walletdb.ErasePool(nIndex); - } + CWalletDB walletdb(*dbw); + walletdb.ErasePool(nIndex); LogPrintf("keypool keep %d\n", nIndex); } @@ -3597,8 +3560,6 @@ bool CWallet::AddDestData(const CTxDestination &dest, const std::string &key, co return false; mapAddressBook[dest].destdata.insert(std::make_pair(key, value)); - if (!fFileBacked) - return true; return CWalletDB(*dbw).WriteDestData(CBitcoinAddress(dest).ToString(), key, value); } @@ -3606,8 +3567,6 @@ bool CWallet::EraseDestData(const CTxDestination &dest, const std::string &key) { if (!mapAddressBook[dest].destdata.erase(key)) return false; - if (!fFileBacked) - return true; return CWalletDB(*dbw).EraseDestData(CBitcoinAddress(dest).ToString(), key); } @@ -3979,8 +3938,6 @@ bool CWallet::ParameterInteraction() bool CWallet::BackupWallet(const std::string& strDest) { - if (!fFileBacked) - return false; return dbw->Backup(strDest); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 318aa2bf97..8cec04e2e7 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -698,8 +698,6 @@ private: /* HD derive new child key (on internal or external chain) */ void DeriveNewChildKey(CKeyMetadata& metadata, CKey& secret, bool internal = false); - bool fFileBacked; - std::set setKeyPool; int64_t nTimeFirstKey; @@ -720,9 +718,7 @@ private: public: /* * Main wallet lock. - * This lock protects all the fields added by CWallet - * except for: - * fFileBacked (immutable after instantiation) + * This lock protects all the fields added by CWallet. */ mutable CCriticalSection cs_wallet; @@ -765,15 +761,16 @@ public: MasterKeyMap mapMasterKeys; unsigned int nMasterKeyMaxID; - CWallet() + // Create wallet with dummy database handle + CWallet(): dbw(new CWalletDBWrapper()) { SetNull(); } + // Create wallet with passed-in database handle CWallet(std::unique_ptr dbw_in) : dbw(std::move(dbw_in)) { SetNull(); - fFileBacked = true; } ~CWallet() @@ -786,7 +783,6 @@ public: { nWalletVersion = FEATURE_BASE; nWalletMaxVersion = FEATURE_BASE; - fFileBacked = false; nMasterKeyMaxID = 0; pwalletdbEncryption = NULL; nOrderPosNext = 0; From be9e1a968debbb7ede8ed50e9288a62ff15d1e1e Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 8 Mar 2017 13:34:47 +0000 Subject: [PATCH 3/6] wallet: Reduce references to global bitdb environment --- src/wallet/db.cpp | 74 ++++++++++++++++++++++++------------------- src/wallet/db.h | 5 +++ src/wallet/wallet.cpp | 2 +- 3 files changed, 48 insertions(+), 33 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 0ea4ea3673..c810476f96 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -364,6 +364,7 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb int ret; fReadOnly = (!strchr(pszMode, '+') && !strchr(pszMode, 'w')); fFlushOnClose = fFlushOnCloseIn; + env = dbw.env; if (dbw.IsDummy()) { return; } @@ -375,17 +376,17 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb nFlags |= DB_CREATE; { - LOCK(bitdb.cs_db); - if (!bitdb.Open(GetDataDir())) + LOCK(env->cs_db); + if (!env->Open(GetDataDir())) throw std::runtime_error("CDB: Failed to open database environment."); strFile = strFilename; - ++bitdb.mapFileUseCount[strFile]; - pdb = bitdb.mapDb[strFile]; + ++env->mapFileUseCount[strFile]; + pdb = env->mapDb[strFile]; if (pdb == NULL) { - pdb = new Db(bitdb.dbenv, 0); + pdb = new Db(env->dbenv, 0); - bool fMockDb = bitdb.IsMock(); + bool fMockDb = env->IsMock(); if (fMockDb) { DbMpoolFile* mpf = pdb->get_mpf(); ret = mpf->set_flags(DB_MPOOL_NOFILE, 1); @@ -403,7 +404,7 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb if (ret != 0) { delete pdb; pdb = NULL; - --bitdb.mapFileUseCount[strFile]; + --env->mapFileUseCount[strFile]; strFile = ""; throw std::runtime_error(strprintf("CDB: Error %d, can't open database %s", ret, strFilename)); } @@ -415,7 +416,7 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb fReadOnly = fTmp; } - bitdb.mapDb[strFile] = pdb; + env->mapDb[strFile] = pdb; } } } @@ -430,7 +431,7 @@ void CDB::Flush() if (fReadOnly) nMinutes = 1; - bitdb.dbenv->txn_checkpoint(nMinutes ? GetArg("-dblogsize", DEFAULT_WALLET_DBLOGSIZE) * 1024 : 0, nMinutes, 0); + env->dbenv->txn_checkpoint(nMinutes ? GetArg("-dblogsize", DEFAULT_WALLET_DBLOGSIZE) * 1024 : 0, nMinutes, 0); } void CDB::Close() @@ -446,8 +447,8 @@ void CDB::Close() Flush(); { - LOCK(bitdb.cs_db); - --bitdb.mapFileUseCount[strFile]; + LOCK(env->cs_db); + --env->mapFileUseCount[strFile]; } } @@ -479,22 +480,23 @@ bool CDB::Rewrite(CWalletDBWrapper& dbw, const char* pszSkip) if (dbw.IsDummy()) { return true; } + CDBEnv *env = dbw.env; const std::string& strFile = dbw.strFile; while (true) { { - LOCK(bitdb.cs_db); - if (!bitdb.mapFileUseCount.count(strFile) || bitdb.mapFileUseCount[strFile] == 0) { + LOCK(env->cs_db); + if (!env->mapFileUseCount.count(strFile) || env->mapFileUseCount[strFile] == 0) { // Flush log data to the dat file - bitdb.CloseDb(strFile); - bitdb.CheckpointLSN(strFile); - bitdb.mapFileUseCount.erase(strFile); + env->CloseDb(strFile); + env->CheckpointLSN(strFile); + env->mapFileUseCount.erase(strFile); bool fSuccess = true; LogPrintf("CDB::Rewrite: Rewriting %s...\n", strFile); std::string strFileRes = strFile + ".rewrite"; { // surround usage of db with extra {} CDB db(dbw, "r"); - Db* pdbCopy = new Db(bitdb.dbenv, 0); + Db* pdbCopy = new Db(env->dbenv, 0); int ret = pdbCopy->open(NULL, // Txn pointer strFileRes.c_str(), // Filename @@ -537,17 +539,17 @@ bool CDB::Rewrite(CWalletDBWrapper& dbw, const char* pszSkip) } if (fSuccess) { db.Close(); - bitdb.CloseDb(strFile); + env->CloseDb(strFile); if (pdbCopy->close(0)) fSuccess = false; delete pdbCopy; } } if (fSuccess) { - Db dbA(bitdb.dbenv, 0); + Db dbA(env->dbenv, 0); if (dbA.remove(strFile.c_str(), NULL, 0)) fSuccess = false; - Db dbB(bitdb.dbenv, 0); + Db dbB(env->dbenv, 0); if (dbB.rename(strFileRes.c_str(), NULL, strFile.c_str(), 0)) fSuccess = false; } @@ -608,14 +610,15 @@ bool CDB::PeriodicFlush(CWalletDBWrapper& dbw) return true; } bool ret = false; + CDBEnv *env = dbw.env; const std::string& strFile = dbw.strFile; TRY_LOCK(bitdb.cs_db,lockDb); if (lockDb) { // Don't do this if any databases are in use int nRefCount = 0; - std::map::iterator mit = bitdb.mapFileUseCount.begin(); - while (mit != bitdb.mapFileUseCount.end()) + std::map::iterator mit = env->mapFileUseCount.begin(); + while (mit != env->mapFileUseCount.end()) { nRefCount += (*mit).second; mit++; @@ -624,17 +627,17 @@ bool CDB::PeriodicFlush(CWalletDBWrapper& dbw) if (nRefCount == 0) { boost::this_thread::interruption_point(); - std::map::iterator mi = bitdb.mapFileUseCount.find(strFile); - if (mi != bitdb.mapFileUseCount.end()) + std::map::iterator mi = env->mapFileUseCount.find(strFile); + if (mi != env->mapFileUseCount.end()) { LogPrint(BCLog::DB, "Flushing %s\n", strFile); int64_t nStart = GetTimeMillis(); // Flush wallet file so it's self contained - bitdb.CloseDb(strFile); - bitdb.CheckpointLSN(strFile); + env->CloseDb(strFile); + env->CheckpointLSN(strFile); - bitdb.mapFileUseCount.erase(mi++); + env->mapFileUseCount.erase(mi++); LogPrint(BCLog::DB, "Flushed %s %dms\n", strFile, GetTimeMillis() - nStart); ret = true; } @@ -657,13 +660,13 @@ bool CWalletDBWrapper::Backup(const std::string& strDest) while (true) { { - LOCK(bitdb.cs_db); - if (!bitdb.mapFileUseCount.count(strFile) || bitdb.mapFileUseCount[strFile] == 0) + LOCK(env->cs_db); + if (!env->mapFileUseCount.count(strFile) || env->mapFileUseCount[strFile] == 0) { // Flush log data to the dat file - bitdb.CloseDb(strFile); - bitdb.CheckpointLSN(strFile); - bitdb.mapFileUseCount.erase(strFile); + env->CloseDb(strFile); + env->CheckpointLSN(strFile); + env->mapFileUseCount.erase(strFile); // Copy wallet file fs::path pathSrc = GetDataDir() / strFile; @@ -685,3 +688,10 @@ bool CWalletDBWrapper::Backup(const std::string& strDest) } return false; } + +void CWalletDBWrapper::Flush(bool shutdown) +{ + if (!IsDummy()) { + env->Flush(shutdown); + } +} diff --git a/src/wallet/db.h b/src/wallet/db.h index 892c8d33c3..dfce3d76cd 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -116,6 +116,10 @@ public: */ std::string GetName() const { return strFile; } + /** Make sure all changes are flushed to disk. + */ + void Flush(bool shutdown); + /** Return whether this database handle is a dummy for testing. * Only to be used at a low level, application should ideally not care * about this. @@ -138,6 +142,7 @@ protected: DbTxn* activeTxn; bool fReadOnly; bool fFlushOnClose; + CDBEnv *env; explicit CDB(CWalletDBWrapper& dbw, const char* pszMode = "r+", bool fFlushOnCloseIn=true); ~CDB() { Close(); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a9d5d02c07..cafb6dda11 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -431,7 +431,7 @@ bool CWallet::HasWalletSpend(const uint256& txid) const void CWallet::Flush(bool shutdown) { - bitdb.Flush(shutdown); + dbw->Flush(shutdown); } bool CWallet::Verify() From 33232810dca802a1328e349865d7da86c8c809e1 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 8 Mar 2017 16:20:08 +0000 Subject: [PATCH 4/6] wallet: CWalletDB CDB composition not inheritance CWalletDB now contains a CDB instead of inheriting from it. This makes it easier to replace the internal transaction with a different database, without leaking through internals. --- src/wallet/db.h | 4 +- src/wallet/walletdb.cpp | 108 ++++++++++++++++++++++++---------------- src/wallet/walletdb.h | 18 ++++++- 3 files changed, 84 insertions(+), 46 deletions(-) diff --git a/src/wallet/db.h b/src/wallet/db.h index dfce3d76cd..014c6db6c5 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -144,10 +144,10 @@ protected: bool fFlushOnClose; CDBEnv *env; +public: explicit CDB(CWalletDBWrapper& dbw, const char* pszMode = "r+", bool fFlushOnCloseIn=true); ~CDB() { Close(); } -public: void Flush(); void Close(); static bool Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue)); @@ -164,7 +164,7 @@ private: CDB(const CDB&); void operator=(const CDB&); -protected: +public: template bool Read(const K& key, T& value) { diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 6921c9900b..a90fa6dbbd 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -33,7 +33,7 @@ static std::atomic nWalletDBUpdateCounter; bool CWalletDB::WriteName(const std::string& strAddress, const std::string& strName) { nWalletDBUpdateCounter++; - return Write(make_pair(std::string("name"), strAddress), strName); + return batch.Write(std::make_pair(std::string("name"), strAddress), strName); } bool CWalletDB::EraseName(const std::string& strAddress) @@ -41,38 +41,38 @@ bool CWalletDB::EraseName(const std::string& strAddress) // This should only be used for sending addresses, never for receiving addresses, // receiving addresses must always have an address book entry if they're not change return. nWalletDBUpdateCounter++; - return Erase(make_pair(std::string("name"), strAddress)); + return batch.Erase(std::make_pair(std::string("name"), strAddress)); } bool CWalletDB::WritePurpose(const std::string& strAddress, const std::string& strPurpose) { nWalletDBUpdateCounter++; - return Write(make_pair(std::string("purpose"), strAddress), strPurpose); + return batch.Write(std::make_pair(std::string("purpose"), strAddress), strPurpose); } bool CWalletDB::ErasePurpose(const std::string& strPurpose) { nWalletDBUpdateCounter++; - return Erase(make_pair(std::string("purpose"), strPurpose)); + return batch.Erase(std::make_pair(std::string("purpose"), strPurpose)); } bool CWalletDB::WriteTx(const CWalletTx& wtx) { nWalletDBUpdateCounter++; - return Write(std::make_pair(std::string("tx"), wtx.GetHash()), wtx); + return batch.Write(std::make_pair(std::string("tx"), wtx.GetHash()), wtx); } bool CWalletDB::EraseTx(uint256 hash) { nWalletDBUpdateCounter++; - return Erase(std::make_pair(std::string("tx"), hash)); + return batch.Erase(std::make_pair(std::string("tx"), hash)); } bool CWalletDB::WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey, const CKeyMetadata& keyMeta) { nWalletDBUpdateCounter++; - if (!Write(std::make_pair(std::string("keymeta"), vchPubKey), + if (!batch.Write(std::make_pair(std::string("keymeta"), vchPubKey), keyMeta, false)) return false; @@ -82,7 +82,7 @@ bool CWalletDB::WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey, c vchKey.insert(vchKey.end(), vchPubKey.begin(), vchPubKey.end()); vchKey.insert(vchKey.end(), vchPrivKey.begin(), vchPrivKey.end()); - return Write(std::make_pair(std::string("key"), vchPubKey), std::make_pair(vchPrivKey, Hash(vchKey.begin(), vchKey.end())), false); + return batch.Write(std::make_pair(std::string("key"), vchPubKey), std::make_pair(vchPrivKey, Hash(vchKey.begin(), vchKey.end())), false); } bool CWalletDB::WriteCryptedKey(const CPubKey& vchPubKey, @@ -92,16 +92,16 @@ bool CWalletDB::WriteCryptedKey(const CPubKey& vchPubKey, const bool fEraseUnencryptedKey = true; nWalletDBUpdateCounter++; - if (!Write(std::make_pair(std::string("keymeta"), vchPubKey), + if (!batch.Write(std::make_pair(std::string("keymeta"), vchPubKey), keyMeta)) return false; - if (!Write(std::make_pair(std::string("ckey"), vchPubKey), vchCryptedSecret, false)) + if (!batch.Write(std::make_pair(std::string("ckey"), vchPubKey), vchCryptedSecret, false)) return false; if (fEraseUnencryptedKey) { - Erase(std::make_pair(std::string("key"), vchPubKey)); - Erase(std::make_pair(std::string("wkey"), vchPubKey)); + batch.Erase(std::make_pair(std::string("key"), vchPubKey)); + batch.Erase(std::make_pair(std::string("wkey"), vchPubKey)); } return true; } @@ -109,92 +109,92 @@ bool CWalletDB::WriteCryptedKey(const CPubKey& vchPubKey, bool CWalletDB::WriteMasterKey(unsigned int nID, const CMasterKey& kMasterKey) { nWalletDBUpdateCounter++; - return Write(std::make_pair(std::string("mkey"), nID), kMasterKey, true); + return batch.Write(std::make_pair(std::string("mkey"), nID), kMasterKey, true); } bool CWalletDB::WriteCScript(const uint160& hash, const CScript& redeemScript) { nWalletDBUpdateCounter++; - return Write(std::make_pair(std::string("cscript"), hash), *(const CScriptBase*)(&redeemScript), false); + return batch.Write(std::make_pair(std::string("cscript"), hash), *(const CScriptBase*)(&redeemScript), false); } bool CWalletDB::WriteWatchOnly(const CScript &dest, const CKeyMetadata& keyMeta) { nWalletDBUpdateCounter++; - if (!Write(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)), keyMeta)) + if (!batch.Write(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)), keyMeta)) return false; - return Write(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest)), '1'); + return batch.Write(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest)), '1'); } bool CWalletDB::EraseWatchOnly(const CScript &dest) { nWalletDBUpdateCounter++; - if (!Erase(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)))) + if (!batch.Erase(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)))) return false; - return Erase(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest))); + return batch.Erase(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest))); } bool CWalletDB::WriteBestBlock(const CBlockLocator& locator) { nWalletDBUpdateCounter++; - Write(std::string("bestblock"), CBlockLocator()); // Write empty block locator so versions that require a merkle branch automatically rescan - return Write(std::string("bestblock_nomerkle"), locator); + batch.Write(std::string("bestblock"), CBlockLocator()); // Write empty block locator so versions that require a merkle branch automatically rescan + return batch.Write(std::string("bestblock_nomerkle"), locator); } bool CWalletDB::ReadBestBlock(CBlockLocator& locator) { - if (Read(std::string("bestblock"), locator) && !locator.vHave.empty()) return true; - return Read(std::string("bestblock_nomerkle"), locator); + if (batch.Read(std::string("bestblock"), locator) && !locator.vHave.empty()) return true; + return batch.Read(std::string("bestblock_nomerkle"), locator); } bool CWalletDB::WriteOrderPosNext(int64_t nOrderPosNext) { nWalletDBUpdateCounter++; - return Write(std::string("orderposnext"), nOrderPosNext); + return batch.Write(std::string("orderposnext"), nOrderPosNext); } bool CWalletDB::WriteDefaultKey(const CPubKey& vchPubKey) { nWalletDBUpdateCounter++; - return Write(std::string("defaultkey"), vchPubKey); + return batch.Write(std::string("defaultkey"), vchPubKey); } bool CWalletDB::ReadPool(int64_t nPool, CKeyPool& keypool) { - return Read(std::make_pair(std::string("pool"), nPool), keypool); + return batch.Read(std::make_pair(std::string("pool"), nPool), keypool); } bool CWalletDB::WritePool(int64_t nPool, const CKeyPool& keypool) { nWalletDBUpdateCounter++; - return Write(std::make_pair(std::string("pool"), nPool), keypool); + return batch.Write(std::make_pair(std::string("pool"), nPool), keypool); } bool CWalletDB::ErasePool(int64_t nPool) { nWalletDBUpdateCounter++; - return Erase(std::make_pair(std::string("pool"), nPool)); + return batch.Erase(std::make_pair(std::string("pool"), nPool)); } bool CWalletDB::WriteMinVersion(int nVersion) { - return Write(std::string("minversion"), nVersion); + return batch.Write(std::string("minversion"), nVersion); } bool CWalletDB::ReadAccount(const std::string& strAccount, CAccount& account) { account.SetNull(); - return Read(make_pair(std::string("acc"), strAccount), account); + return batch.Read(std::make_pair(std::string("acc"), strAccount), account); } bool CWalletDB::WriteAccount(const std::string& strAccount, const CAccount& account) { - return Write(make_pair(std::string("acc"), strAccount), account); + return batch.Write(std::make_pair(std::string("acc"), strAccount), account); } bool CWalletDB::WriteAccountingEntry(const uint64_t nAccEntryNum, const CAccountingEntry& acentry) { - return Write(std::make_pair(std::string("acentry"), std::make_pair(acentry.strAccount, nAccEntryNum)), acentry); + return batch.Write(std::make_pair(std::string("acentry"), std::make_pair(acentry.strAccount, nAccEntryNum)), acentry); } bool CWalletDB::WriteAccountingEntry_Backend(const CAccountingEntry& acentry) @@ -218,7 +218,7 @@ void CWalletDB::ListAccountCreditDebit(const std::string& strAccount, std::list< { bool fAllAccounts = (strAccount == "*"); - Dbc* pcursor = GetCursor(); + Dbc* pcursor = batch.GetCursor(); if (!pcursor) throw std::runtime_error(std::string(__func__) + ": cannot create DB cursor"); bool setRange = true; @@ -229,7 +229,7 @@ void CWalletDB::ListAccountCreditDebit(const std::string& strAccount, std::list< if (setRange) ssKey << std::make_pair(std::string("acentry"), std::make_pair((fAllAccounts ? std::string("") : strAccount), uint64_t(0))); CDataStream ssValue(SER_DISK, CLIENT_VERSION); - int ret = ReadAtCursor(pcursor, ssKey, ssValue, setRange); + int ret = batch.ReadAtCursor(pcursor, ssKey, ssValue, setRange); setRange = false; if (ret == DB_NOTFOUND) break; @@ -560,7 +560,7 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet) LOCK(pwallet->cs_wallet); try { int nMinVersion = 0; - if (Read((std::string)"minversion", nMinVersion)) + if (batch.Read((std::string)"minversion", nMinVersion)) { if (nMinVersion > CLIENT_VERSION) return DB_TOO_NEW; @@ -568,7 +568,7 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet) } // Get cursor - Dbc* pcursor = GetCursor(); + Dbc* pcursor = batch.GetCursor(); if (!pcursor) { LogPrintf("Error getting wallet database cursor\n"); @@ -580,7 +580,7 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet) // Read next record CDataStream ssKey(SER_DISK, CLIENT_VERSION); CDataStream ssValue(SER_DISK, CLIENT_VERSION); - int ret = ReadAtCursor(pcursor, ssKey, ssValue); + int ret = batch.ReadAtCursor(pcursor, ssKey, ssValue); if (ret == DB_NOTFOUND) break; else if (ret != 0) @@ -664,14 +664,14 @@ DBErrors CWalletDB::FindWalletTx(std::vector& vTxHash, std::vector CLIENT_VERSION) return DB_TOO_NEW; } // Get cursor - Dbc* pcursor = GetCursor(); + Dbc* pcursor = batch.GetCursor(); if (!pcursor) { LogPrintf("Error getting wallet database cursor\n"); @@ -683,7 +683,7 @@ DBErrors CWalletDB::FindWalletTx(std::vector& vTxHash, std::vector Date: Thu, 20 Apr 2017 17:52:47 +0200 Subject: [PATCH 5/6] wallet: Make IsDummy private in CWalletDBWrapper This is only for use in the low-level functions, and CDB is already a friend class. --- src/wallet/db.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/wallet/db.h b/src/wallet/db.h index 014c6db6c5..a0719820ac 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -120,16 +120,16 @@ public: */ void Flush(bool shutdown); +private: + /** BerkeleyDB specific */ + CDBEnv *env; + std::string strFile; + /** Return whether this database handle is a dummy for testing. * Only to be used at a low level, application should ideally not care * about this. */ bool IsDummy() { return env == nullptr; } - -private: - /** BerkeleyDB specific */ - CDBEnv *env; - std::string strFile; }; From 911a4808fb8d9c73463f9686ca4c26b8556ce53b Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 21 Apr 2017 16:04:26 +0200 Subject: [PATCH 6/6] wallet: Add comment describing the various classes in walletdb.h --- src/wallet/walletdb.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 98a65a130c..cd9fe279c5 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -17,6 +17,21 @@ #include #include +/** + * Overview of wallet database classes: + * + * - CDBEnv is an environment in which the database exists (has no analog in dbwrapper.h) + * - CWalletDBWrapper represents a wallet database (similar to CDBWrapper in dbwrapper.h) + * - CDB is a low-level database transaction (similar to CDBBatch in dbwrapper.h) + * - CWalletDB is a modifier object for the wallet, and encapsulates a database + * transaction as well as methods to act on the database (no analog in + * dbwrapper.h) + * + * The latter two are named confusingly, in contrast to what the names CDB + * and CWalletDB suggest they are transient transaction objects and don't + * represent the database itself. + */ + static const bool DEFAULT_FLUSHWALLET = true; class CAccount;