From a1bef4f500cb309322157663a1b141336b536bab Mon Sep 17 00:00:00 2001 From: Dan Raviv Date: Sat, 16 Sep 2017 13:06:05 +0300 Subject: [PATCH 01/32] Refactor: Modernize disallowed copy constructors/assignment Use C++11's better capability of expressing an interface of a non-copyable class by publicly deleting its copy ctor and assignment operator instead of just declaring them private. --- src/coins.h | 10 +++++----- src/net.h | 6 ++---- src/streams.h | 16 ++++++++-------- src/support/lockedpool.h | 12 ++++++------ src/txdb.h | 6 ++---- src/wallet/db.h | 7 +++---- src/wallet/walletdb.h | 5 ++--- 7 files changed, 28 insertions(+), 34 deletions(-) diff --git a/src/coins.h b/src/coins.h index d7eef36fc7be..287a89677acb 100644 --- a/src/coins.h +++ b/src/coins.h @@ -300,6 +300,11 @@ class CCoinsViewCache : public CCoinsViewBacked public: CCoinsViewCache(CCoinsView *baseIn); + /** + * By deleting the copy constructor, we prevent accidentally using it when one intends to create a cache on top of a base cache. + */ + CCoinsViewCache(const CCoinsViewCache &) = delete; + // Sapling methods bool GetSaplingAnchorAt(const uint256 &rt, SaplingMerkleTree &tree) const override; bool GetNullifier(const uint256 &nullifier) const override; @@ -430,11 +435,6 @@ class CCoinsViewCache : public CCoinsViewBacked const uint256 ¤tRoot, Tree &tree ); - - /** - * By making the copy constructor private, we prevent accidentally using it when one intends to create a cache on top of a base cache. - */ - CCoinsViewCache(const CCoinsViewCache &); }; //! Utility function to add all of a transaction's outputs to a cache. diff --git a/src/net.h b/src/net.h index 480d9265179c..df24e888d52d 100644 --- a/src/net.h +++ b/src/net.h @@ -631,12 +631,10 @@ class CNode CNode(NodeId id, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const std::string& addrNameIn = "", bool fInboundIn = false); ~CNode(); + CNode(const CNode&) = delete; + CNode& operator=(const CNode&) = delete; private: - CNode(const CNode&); - void operator=(const CNode&); - - const uint64_t nLocalHostNonce; // Services offered to this peer const ServiceFlags nLocalServices; diff --git a/src/streams.h b/src/streams.h index 9a2198aa030b..baa30dc04832 100644 --- a/src/streams.h +++ b/src/streams.h @@ -408,10 +408,6 @@ class CDataStream : public CBaseDataStream class CAutoFile { private: - // Disallow copies - CAutoFile(const CAutoFile&); - CAutoFile& operator=(const CAutoFile&); - const int nType; const int nVersion; @@ -428,6 +424,10 @@ class CAutoFile fclose(); } + // Disallow copies + CAutoFile(const CAutoFile&) = delete; + CAutoFile& operator=(const CAutoFile&) = delete; + void fclose() { if (file) { @@ -523,10 +523,6 @@ class CAutoFile class CBufferedFile { private: - // Disallow copies - CBufferedFile(const CBufferedFile&); - CBufferedFile& operator=(const CBufferedFile&); - const int nType; const int nVersion; @@ -569,6 +565,10 @@ class CBufferedFile fclose(); } + // Disallow copies + CBufferedFile(const CBufferedFile&) = delete; + CBufferedFile& operator=(const CBufferedFile&) = delete; + void fclose() { if (src) { diff --git a/src/support/lockedpool.h b/src/support/lockedpool.h index 526c17a73fe7..d16f7e9f4ff3 100644 --- a/src/support/lockedpool.h +++ b/src/support/lockedpool.h @@ -50,6 +50,9 @@ class Arena Arena(void *base, size_t size, size_t alignment); virtual ~Arena(); + Arena(const Arena& other) = delete; // non construction-copyable + Arena& operator=(const Arena&) = delete; // non copyable + /** A chunk of memory. */ struct Chunk @@ -106,9 +109,6 @@ class Arena */ bool addressInArena(void *ptr) const { return ptr >= base && ptr < end; } private: - Arena(const Arena& other) = delete; // non construction-copyable - Arena& operator=(const Arena&) = delete; // non copyable - /** Map of chunk address to chunk information. This class makes use of the * sorted order to merge previous and next chunks during deallocation. */ @@ -173,6 +173,9 @@ class LockedPool LockedPool(std::unique_ptr allocator, LockingFailed_Callback lf_cb_in = 0); ~LockedPool(); + LockedPool(const LockedPool& other) = delete; // non construction-copyable + LockedPool& operator=(const LockedPool&) = delete; // non copyable + /** Allocate size bytes from this arena. * Returns pointer on success, or 0 if memory is full or * the application tried to allocate 0 bytes. @@ -188,9 +191,6 @@ class LockedPool /** Get pool usage statistics */ Stats stats() const; private: - LockedPool(const LockedPool& other) = delete; // non construction-copyable - LockedPool& operator=(const LockedPool&) = delete; // non copyable - std::unique_ptr allocator; /** Create an arena from locked pages */ diff --git a/src/txdb.h b/src/txdb.h index 9b2604beff17..99fbb2e22b6b 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -129,11 +129,9 @@ class CBlockTreeDB : public CDBWrapper public: CBlockTreeDB(size_t nCacheSize, bool fMemory = false, bool fWipe = false); -private: - CBlockTreeDB(const CBlockTreeDB&); - void operator=(const CBlockTreeDB&); + CBlockTreeDB(const CBlockTreeDB&) = delete; + CBlockTreeDB& operator=(const CBlockTreeDB&) = delete; -public: bool WriteBlockIndex(const CDiskBlockIndex& blockindex); bool WriteBatchSync(const std::vector >& fileInfo, int nLastFile, const std::vector& blockinfo); bool ReadBlockFileInfo(int nFile, CBlockFileInfo& fileinfo); diff --git a/src/wallet/db.h b/src/wallet/db.h index 413e98dd7b66..948b9ffa8af8 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -158,6 +158,9 @@ class CDB explicit CDB(CWalletDBWrapper& dbw, const char* pszMode = "r+", bool fFlushOnCloseIn=true); ~CDB() { Close(); } + CDB(const CDB&) = delete; + CDB& operator=(const CDB&) = delete; + void Flush(); void Close(); static bool Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); @@ -170,10 +173,6 @@ class CDB /* verifies the database file */ static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& dataDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc); -private: - CDB(const CDB&); - void operator=(const CDB&); - public: template bool Read(const K& key, T& value) diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index c65f1c099372..4ad62d9b7502 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -136,6 +136,8 @@ class CWalletDB m_dbw(dbw) { } + CWalletDB(const CWalletDB&) = delete; + CWalletDB& operator=(const CWalletDB&) = delete; bool WriteName(const std::string& strAddress, const std::string& strName); bool EraseName(const std::string& strAddress); @@ -227,9 +229,6 @@ class CWalletDB private: CDB batch; CWalletDBWrapper& m_dbw; - - CWalletDB(const CWalletDB&); - void operator=(const CWalletDB&); }; void NotifyBacked(const CWallet& wallet, bool fSuccess, std::string strMessage); From 5b31813938356152e721396cea00b08ea62c3e7d Mon Sep 17 00:00:00 2001 From: practicalswift Date: Wed, 9 Aug 2017 16:24:12 +0200 Subject: [PATCH 02/32] Use unique_ptr for dbenv (DbEnv) --- src/wallet/db.cpp | 31 ++++++++++++++----------------- src/wallet/db.h | 2 +- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 9f1d0c095ead..11fb68278ba4 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -77,13 +77,12 @@ void CDBEnv::EnvShutdown() void CDBEnv::Reset() { - delete dbenv; - dbenv = new DbEnv(DB_CXX_NO_EXCEPTIONS); + dbenv.reset(new DbEnv(DB_CXX_NO_EXCEPTIONS)); fDbEnvInit = false; fMockDb = false; } -CDBEnv::CDBEnv() : dbenv(NULL) +CDBEnv::CDBEnv() { Reset(); } @@ -91,8 +90,6 @@ CDBEnv::CDBEnv() : dbenv(NULL) CDBEnv::~CDBEnv() { EnvShutdown(); - delete dbenv; - dbenv = NULL; } void CDBEnv::Close() @@ -184,8 +181,8 @@ CDBEnv::VerifyResult CDBEnv::Verify(const std::string& strFile, recoverFunc_type LOCK(cs_db); assert(mapFileUseCount.count(strFile) == 0); - Db db(dbenv, 0); - int result = db.verify(strFile.c_str(), NULL, NULL, 0); + Db db(dbenv.get(), 0); + int result = db.verify(strFile.c_str(), nullptr, nullptr, 0); if (result == 0) return VERIFY_OK; else if (recoverFunc == NULL) @@ -225,8 +222,8 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco } LogPrintf("Salvage(aggressive) found %u records\n", salvagedData.size()); - std::unique_ptr pdbCopy(new Db(bitdb.dbenv, 0)); - int ret = pdbCopy->open(NULL, // Txn pointer + std::unique_ptr pdbCopy(new Db(bitdb.dbenv.get(), 0)); + int ret = pdbCopy->open(nullptr, // Txn pointer filename.c_str(), // Filename "main", // Logical db name DB_BTREE, // Database type @@ -326,8 +323,8 @@ bool CDBEnv::Salvage(const std::string& strFile, bool fAggressive, std::vectormapDb[strFilename]; if (pdb == nullptr) { int ret; - std::unique_ptr pdb_temp(new Db(env->dbenv, 0)); + std::unique_ptr pdb_temp(new Db(env->dbenv.get(), 0)); bool fMockDb = env->IsMock(); if (fMockDb) { @@ -517,7 +514,7 @@ bool CDB::Rewrite(CWalletDBWrapper& dbw, const char* pszSkip) std::string strFileRes = strFile + ".rewrite"; { // surround usage of db with extra {} CDB db(dbw, "r"); - Db* pdbCopy = new Db(env->dbenv, 0); + Db* pdbCopy = new Db(env->dbenv.get(), 0); int ret = pdbCopy->open(NULL, // Txn pointer strFileRes.c_str(), // Filename @@ -569,11 +566,11 @@ bool CDB::Rewrite(CWalletDBWrapper& dbw, const char* pszSkip) delete pdbCopy; } if (fSuccess) { - Db dbA(env->dbenv, 0); - if (dbA.remove(strFile.c_str(), NULL, 0)) + Db dbA(env->dbenv.get(), 0); + if (dbA.remove(strFile.c_str(), nullptr, 0)) fSuccess = false; - Db dbB(env->dbenv, 0); - if (dbB.rename(strFileRes.c_str(), NULL, strFile.c_str(), 0)) + Db dbB(env->dbenv.get(), 0); + if (dbB.rename(strFileRes.c_str(), nullptr, strFile.c_str(), 0)) fSuccess = false; } if (!fSuccess) diff --git a/src/wallet/db.h b/src/wallet/db.h index 948b9ffa8af8..a3e0c288cdef 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -37,7 +37,7 @@ class CDBEnv public: mutable RecursiveMutex cs_db; - DbEnv *dbenv; + std::unique_ptr dbenv; std::map mapFileUseCount; std::map mapDb; From 71a4701a7311c22a2eeb06ce1d7047109b9d4f92 Mon Sep 17 00:00:00 2001 From: furszy Date: Mon, 14 Jun 2021 23:53:27 -0300 Subject: [PATCH 03/32] Add -walletdir parameter to specify custom wallet dir Adaptation of btc@0530ba0eae147563921b162ed05347234d8b53c0 --- CMakeLists.txt | 1 + src/Makefile.am | 2 ++ src/init.cpp | 2 ++ src/pivxd.cpp | 9 ++++++++- src/qt/pivx.cpp | 6 ++++++ src/wallet/db.cpp | 27 +++++++++++++++------------ src/wallet/db.h | 4 ++-- src/wallet/init.cpp | 8 +++++--- src/wallet/rpcwallet.cpp | 1 + src/wallet/walletdb.cpp | 8 ++++---- src/wallet/walletdb.h | 4 ++-- src/wallet/walletutil.cpp | 24 ++++++++++++++++++++++++ src/wallet/walletutil.h | 13 +++++++++++++ 13 files changed, 85 insertions(+), 24 deletions(-) create mode 100644 src/wallet/walletutil.cpp create mode 100644 src/wallet/walletutil.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 46c3e6baf63b..7764a05f1c6a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -267,6 +267,7 @@ set(WALLET_SOURCES ./src/legacy/stakemodifier.cpp ./src/wallet/wallet.cpp ./src/wallet/walletdb.cpp + ./src/wallet/walletutil.cpp ./src/zpiv/zpivmodule.cpp ./src/zpiv/zpos.cpp ./src/stakeinput.cpp diff --git a/src/Makefile.am b/src/Makefile.am index f433e718da77..cdf0e323ed98 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -305,6 +305,7 @@ BITCOIN_CORE_H = \ wallet/init.h \ wallet/wallet.h \ wallet/walletdb.h \ + wallet/walletutil.h \ warnings.h \ zpivchain.h \ zpiv/zpos.h \ @@ -408,6 +409,7 @@ libbitcoin_wallet_a_SOURCES = \ destination_io.cpp \ wallet/wallet.cpp \ wallet/walletdb.cpp \ + wallet/walletutil.cpp \ stakeinput.cpp \ zpiv/zpos.cpp \ $(BITCOIN_CORE_H) \ diff --git a/src/init.cpp b/src/init.cpp index 9a80c481892b..6191de8a1d63 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -64,6 +64,7 @@ #include "wallet/init.h" #include "wallet/wallet.h" #include "wallet/rpcwallet.h" +#include "wallet/walletutil.h" #endif #include @@ -1268,6 +1269,7 @@ bool AppInitMain() LogPrintf("Startup time: %s\n", FormatISO8601DateTime(GetTime())); LogPrintf("Default data directory %s\n", GetDefaultDataDir().string()); LogPrintf("Using data directory %s\n", GetDataDir().string()); + LogPrintf("Using wallet directory %s\n", GetWalletDir().string()); LogPrintf("Using config file %s\n", GetConfigFile(gArgs.GetArg("-conf", PIVX_CONF_FILENAME)).string()); LogPrintf("Using at most %i connections (%i file descriptors available)\n", nMaxConnections, nFD); std::ostringstream strErrors; diff --git a/src/pivxd.cpp b/src/pivxd.cpp index ad02677ccc8f..3407a07ec10d 100644 --- a/src/pivxd.cpp +++ b/src/pivxd.cpp @@ -11,9 +11,12 @@ #include "init.h" #include "masternodeconfig.h" #include "noui.h" -#include "rpc/server.h" #include "util/system.h" +#ifdef ENABLE_WALLET +#include "wallet/walletutil.h" +#endif + #include /* Introduction text for doxygen: */ @@ -81,6 +84,10 @@ bool AppInit(int argc, char* argv[]) fprintf(stderr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "").c_str()); return false; } + if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) { + fprintf(stderr, "Error: Specified wallet directory \"%s\" does not exist.\n", gArgs.GetArg("-walletdir", "").c_str()); + return false; + } try { gArgs.ReadConfigFile(gArgs.GetArg("-conf", PIVX_CONF_FILENAME)); } catch (const std::exception& e) { diff --git a/src/qt/pivx.cpp b/src/qt/pivx.cpp index 5a0e35dd0ced..218f6573e5dd 100644 --- a/src/qt/pivx.cpp +++ b/src/qt/pivx.cpp @@ -26,6 +26,7 @@ #include "paymentserver.h" #include "walletmodel.h" #include "interfaces/wallet.h" +#include "wallet/walletutil.h" #endif #include "masternodeconfig.h" @@ -607,6 +608,11 @@ int main(int argc, char* argv[]) QObject::tr("Error: Specified data directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-datadir", "")))); return 1; } + if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) { + QMessageBox::critical(0, QObject::tr(PACKAGE_NAME), + QObject::tr("Error: Specified wallet directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-walletdir", "")))); + return EXIT_FAILURE; + } try { gArgs.ReadConfigFile(gArgs.GetArg("-conf", PIVX_CONF_FILENAME)); } catch (const std::exception& e) { diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 11fb68278ba4..f0bc8e512396 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -12,6 +12,7 @@ #include "protocol.h" #include "util/system.h" #include "utilstrencodings.h" +#include "wallet/walletutil.h" #include @@ -255,7 +256,7 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco return fSuccess; } -bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& dataDir, std::string& errorStr) +bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr) { LogPrintf("Using BerkeleyDB version %s\n", DbEnv::version(0, 0, 0)); LogPrintf("Using wallet %s\n", walletFile); @@ -263,12 +264,13 @@ bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& dataD // Wallet file must be a plain filename without a directory fs::path wallet_file_path(walletFile); if (walletFile != wallet_file_path.filename().string()) - return UIError(strprintf(_("Wallet %s resides outside data directory %s"), walletFile, dataDir.string())); + return UIError(strprintf(_("Wallet %s resides outside data directory %s"), walletFile, walletDir.string())); - if (!bitdb.Open(dataDir)) { + if (!bitdb.Open(walletDir)) + { // try moving the database env out of the way - fs::path pathDatabase = dataDir / "database"; - fs::path pathDatabaseBak = dataDir / strprintf("database.%d.bak", GetTime()); + fs::path pathDatabase = walletDir / "database"; + fs::path pathDatabaseBak = walletDir / strprintf("database.%d.bak", GetTime()); try { fs::rename(pathDatabase, pathDatabaseBak); LogPrintf("Moved old %s to %s. Retrying.\n", pathDatabase.string(), pathDatabaseBak.string()); @@ -277,18 +279,19 @@ bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& dataD } // try again - if (!bitdb.Open(dataDir)) { + if (!bitdb.Open(walletDir)) { // if it still fails, it probably means we can't even create the database env - errorStr = strprintf(_("Error initializing wallet database environment %s!"), GetDataDir()); + errorStr = strprintf(_("Error initializing wallet database environment %s!"), walletDir); return false; } } return true; } -bool CDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& dataDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc) +bool CDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc) { - if (fs::exists(dataDir / walletFile)) { + if (fs::exists(walletDir / walletFile)) + { std::string backup_filename; CDBEnv::VerifyResult r = bitdb.Verify(walletFile, recoverFunc, backup_filename); if (r == CDBEnv::RECOVER_OK) { @@ -296,7 +299,7 @@ bool CDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& data " Original %s saved as %s in %s; if" " your balance or transactions are incorrect you should" " restore from a backup."), - walletFile, backup_filename, dataDir); + walletFile, backup_filename, walletDir); } if (r == CDBEnv::RECOVER_FAIL) { errorStr = strprintf(_("%s corrupt, salvage failed"), walletFile); @@ -399,7 +402,7 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb { LOCK(env->cs_db); - if (!env->Open(GetDataDir())) + if (!env->Open(GetWalletDir())) throw std::runtime_error("CDB: Failed to open database environment."); pdb = env->mapDb[strFilename]; @@ -686,7 +689,7 @@ bool CWalletDBWrapper::Backup(const std::string& strDest) env->mapFileUseCount.erase(strFile); // Copy wallet file - fs::path pathSrc = GetDataDir() / strFile; + fs::path pathSrc = GetWalletDir() / strFile; fs::path pathDest(strDest); if (fs::is_directory(pathDest)) pathDest /= strFile; diff --git a/src/wallet/db.h b/src/wallet/db.h index a3e0c288cdef..168f6522f986 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -169,9 +169,9 @@ class CDB ideal to be called periodically */ static bool PeriodicFlush(CWalletDBWrapper& dbw); /* verifies the database environment */ - static bool VerifyEnvironment(const std::string& walletFile, const fs::path& dataDir, std::string& errorStr); + static bool VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr); /* verifies the database file */ - static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& dataDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc); + static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc); public: template diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index f7bfcf90e3c7..d2caf86fab8e 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -12,6 +12,7 @@ #include "utilmoneystr.h" #include "validation.h" #include "wallet/wallet.h" +#include "wallet/walletutil.h" std::string GetWalletHelpString(bool showDebug) { @@ -31,6 +32,7 @@ std::string GetWalletHelpString(bool showDebug) strUsage += HelpMessageOpt("-txconfirmtarget=", strprintf(_("If paytxfee is not set, include enough fee so transactions begin confirmation on average within n blocks (default: %u)"), 1)); strUsage += HelpMessageOpt("-upgradewallet", _("Upgrade wallet to latest format") + " " + _("on startup")); strUsage += HelpMessageOpt("-wallet=", _("Specify wallet file (within data directory)") + " " + strprintf(_("(default: %s)"), DEFAULT_WALLET_DAT)); + strUsage += HelpMessageOpt("-walletdir=", _("Specify directory to hold wallets (default: /wallets if it exists, otherwise )")); strUsage += HelpMessageOpt("-walletnotify=", _("Execute command when a wallet transaction changes (%s in cmd is replaced by TxID)")); strUsage += HelpMessageOpt("-zapwallettxes=", _("Delete all wallet transactions and only recover those parts of the blockchain through -rescan on startup") + " " + _("(1 = keep tx meta data e.g. payment request information, 2 = drop tx meta data)")); @@ -159,7 +161,7 @@ bool WalletVerify() return UIError(strprintf(_("Error loading wallet %s. Invalid characters in %s filename."), walletFile, "-wallet")); } - fs::path wallet_path = fs::absolute(walletFile, GetDataDir()); + fs::path wallet_path = fs::absolute(walletFile, GetWalletDir()); if (fs::exists(wallet_path) && (!fs::is_regular_file(wallet_path) || fs::is_symlink(wallet_path))) { return UIError(strprintf(_("Error loading wallet %s. %s filename must be a regular file."), walletFile, "-wallet")); @@ -170,7 +172,7 @@ bool WalletVerify() } std::string strError; - if (!CWalletDB::VerifyEnvironment(walletFile, GetDataDir().string(), strError)) { + if (!CWalletDB::VerifyEnvironment(walletFile, GetWalletDir().string(), strError)) { return UIError(strError); } @@ -189,7 +191,7 @@ bool WalletVerify() } std::string strWarning; - bool dbV = CWalletDB::VerifyDatabaseFile(walletFile, GetDataDir().string(), strWarning, strError); + bool dbV = CWalletDB::VerifyDatabaseFile(walletFile, GetWalletDir().string(), strWarning, strError); if (!strWarning.empty()) { UIWarning(strWarning); } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 7b683d0f23af..0f100f2f99d5 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -27,6 +27,7 @@ #include "utilmoneystr.h" #include "wallet/wallet.h" #include "wallet/walletdb.h" +#include "wallet/walletutil.h" #include "zpivchain.h" #include // for StartShutdown diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 1171e25be653..fece219f49e3 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -1064,14 +1064,14 @@ bool CWalletDB::RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDa return true; } -bool CWalletDB::VerifyEnvironment(const std::string& walletFile, const fs::path& dataDir, std::string& errorStr) +bool CWalletDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr) { - return CDB::VerifyEnvironment(walletFile, dataDir, errorStr); + return CDB::VerifyEnvironment(walletFile, walletDir, errorStr); } -bool CWalletDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& dataDir, std::string& warningStr, std::string& errorStr) +bool CWalletDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr) { - return CDB::VerifyDatabaseFile(walletFile, dataDir, warningStr, errorStr, CWalletDB::Recover); + return CDB::VerifyDatabaseFile(walletFile, walletDir, warningStr, errorStr, CWalletDB::Recover); } bool CWalletDB::WriteDestData(const std::string& address, const std::string& key, const std::string& value) diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 4ad62d9b7502..c0803404ff3a 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -212,9 +212,9 @@ class CWalletDB /* 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 fs::path& dataDir, std::string& errorStr); + static bool VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr); /* verifies the database file */ - static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& dataDir, std::string& warningStr, std::string& errorStr); + static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr); //! Begin a new transaction bool TxnBegin(); diff --git a/src/wallet/walletutil.cpp b/src/wallet/walletutil.cpp new file mode 100644 index 000000000000..edf2eb323dc3 --- /dev/null +++ b/src/wallet/walletutil.cpp @@ -0,0 +1,24 @@ +// Copyright (c) 2017 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "wallet/walletutil.h" +#include "util/system.h" + +fs::path GetWalletDir() +{ + fs::path path; + + if (gArgs.IsArgSet("-walletdir")) { + path = fs::system_complete(gArgs.GetArg("-walletdir", "")); + if (!fs::is_directory(path)) { + // If the path specified doesn't exist, we return the deliberately + // invalid empty string. + path = ""; + } + } else { + path = GetDataDir(); + } + + return path; +} diff --git a/src/wallet/walletutil.h b/src/wallet/walletutil.h new file mode 100644 index 000000000000..b1a232f494df --- /dev/null +++ b/src/wallet/walletutil.h @@ -0,0 +1,13 @@ +// Copyright (c) 2017 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_WALLET_UTIL_H +#define BITCOIN_WALLET_UTIL_H + +#include "fs.h" + +//! Get the path of the wallet directory. +fs::path GetWalletDir(); + +#endif // BITCOIN_WALLET_UTIL_H From 359b01da2b4fef9f8b5c3991df20cb283357ef1c Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Mon, 9 Oct 2017 21:45:01 +1300 Subject: [PATCH 04/32] Add release notes for -walletdir and wallets/ dir --- doc/release-notes.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/doc/release-notes.md b/doc/release-notes.md index 283a76a5aa38..f5eee78edd01 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -299,6 +299,19 @@ The `getwalletinfo` RPC method returns the name of the wallet used for the query Note that while multi-wallet is now fully supported, the RPC multi-wallet interface should be considered unstable for version 6.0.0, and there may backwards-incompatible changes in future versions. +Custom wallet directories +--------------------- +The ability to specify a directory other than the default data directory in which to store +wallets has been added. An existing directory can be specified using the `-walletdir=` +argument. Wallets loaded via `-wallet` arguments must be in this wallet directory. Care should be taken +when choosing a wallet directory location, as if it becomes unavailable during operation, +funds may be lost. + +Default wallet directory change +-------------------------- +On new installations (if the data directory doesn't exist), wallets will now be stored in a +new `wallets/` subdirectory inside the data directory. If this `wallets/` subdirectory +doesn't exist (i.e. on existing nodes), the current datadir root is used instead, as it was. Database cache memory increased -------------------------------- From 03db5c89e54b933fb7d855e1f8af07322300a4db Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Fri, 27 Oct 2017 15:15:40 +1300 Subject: [PATCH 05/32] Default walletdir is wallets/ if it exists --- src/wallet/walletutil.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/wallet/walletutil.cpp b/src/wallet/walletutil.cpp index edf2eb323dc3..2f47d911640a 100644 --- a/src/wallet/walletutil.cpp +++ b/src/wallet/walletutil.cpp @@ -18,6 +18,10 @@ fs::path GetWalletDir() } } else { path = GetDataDir(); + // If a wallets directory exists, use that, otherwise default to GetDataDir + if (fs::is_directory(path / "wallets")) { + path /= "wallets"; + } } return path; From dcb43eaf61b41c60e577a6e85ee13e3bad4e61ee Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 15 Jun 2021 02:13:35 -0300 Subject: [PATCH 06/32] Create walletdir if datadir doesn't exist and correct tests Adaptation of btc@8263f6a5ac3f3af102a2819b7e179b00db7e0437 And cleanups for walletdir PR Adaptation of btc@b67342906ced2353d378f4369c8d8a979d525fee --- doc/release-notes.md | 2 +- src/init.cpp | 2 -- src/pivxd.cpp | 8 -------- src/qt/intro.cpp | 5 ++++- src/qt/pivx.cpp | 5 ----- src/util/system.cpp | 5 ++++- src/wallet/init.cpp | 9 +++++++++ src/wallet/walletdb.cpp | 3 ++- test/functional/wallet_backup.py | 20 ++++++++++---------- test/functional/wallet_hd.py | 6 +++--- test/functional/wallet_keypool_topup.py | 4 ++-- test/functional/wallet_multiwallet.py | 25 +++++++++++++------------ 12 files changed, 48 insertions(+), 46 deletions(-) diff --git a/doc/release-notes.md b/doc/release-notes.md index f5eee78edd01..62d81590fc4b 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -304,7 +304,7 @@ Custom wallet directories The ability to specify a directory other than the default data directory in which to store wallets has been added. An existing directory can be specified using the `-walletdir=` argument. Wallets loaded via `-wallet` arguments must be in this wallet directory. Care should be taken -when choosing a wallet directory location, as if it becomes unavailable during operation, +when choosing a wallet directory location, as if it becomes unavailable during operation, funds may be lost. Default wallet directory change diff --git a/src/init.cpp b/src/init.cpp index 6191de8a1d63..9a80c481892b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -64,7 +64,6 @@ #include "wallet/init.h" #include "wallet/wallet.h" #include "wallet/rpcwallet.h" -#include "wallet/walletutil.h" #endif #include @@ -1269,7 +1268,6 @@ bool AppInitMain() LogPrintf("Startup time: %s\n", FormatISO8601DateTime(GetTime())); LogPrintf("Default data directory %s\n", GetDefaultDataDir().string()); LogPrintf("Using data directory %s\n", GetDataDir().string()); - LogPrintf("Using wallet directory %s\n", GetWalletDir().string()); LogPrintf("Using config file %s\n", GetConfigFile(gArgs.GetArg("-conf", PIVX_CONF_FILENAME)).string()); LogPrintf("Using at most %i connections (%i file descriptors available)\n", nMaxConnections, nFD); std::ostringstream strErrors; diff --git a/src/pivxd.cpp b/src/pivxd.cpp index 3407a07ec10d..be30d3aae6c6 100644 --- a/src/pivxd.cpp +++ b/src/pivxd.cpp @@ -13,10 +13,6 @@ #include "noui.h" #include "util/system.h" -#ifdef ENABLE_WALLET -#include "wallet/walletutil.h" -#endif - #include /* Introduction text for doxygen: */ @@ -84,10 +80,6 @@ bool AppInit(int argc, char* argv[]) fprintf(stderr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "").c_str()); return false; } - if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) { - fprintf(stderr, "Error: Specified wallet directory \"%s\" does not exist.\n", gArgs.GetArg("-walletdir", "").c_str()); - return false; - } try { gArgs.ReadConfigFile(gArgs.GetArg("-conf", PIVX_CONF_FILENAME)); } catch (const std::exception& e) { diff --git a/src/qt/intro.cpp b/src/qt/intro.cpp index 28f8a1e99fde..c8485cdb14ea 100644 --- a/src/qt/intro.cpp +++ b/src/qt/intro.cpp @@ -193,7 +193,10 @@ bool Intro::pickDataDirectory() } dataDir = intro.getDataDirectory(); try { - TryCreateDirectories(GUIUtil::qstringToBoostPath(dataDir)); + if (TryCreateDirectories(GUIUtil::qstringToBoostPath(dataDir))) { + // If a new data directory has been created, make wallets subdirectory too + TryCreateDirectories(GUIUtil::qstringToBoostPath(dataDir) / "wallets"); + } break; } catch (const fs::filesystem_error& e) { QMessageBox::critical(0, tr("PIVX Core"), diff --git a/src/qt/pivx.cpp b/src/qt/pivx.cpp index 218f6573e5dd..ccd933a1d4e5 100644 --- a/src/qt/pivx.cpp +++ b/src/qt/pivx.cpp @@ -608,11 +608,6 @@ int main(int argc, char* argv[]) QObject::tr("Error: Specified data directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-datadir", "")))); return 1; } - if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) { - QMessageBox::critical(0, QObject::tr(PACKAGE_NAME), - QObject::tr("Error: Specified wallet directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-walletdir", "")))); - return EXIT_FAILURE; - } try { gArgs.ReadConfigFile(gArgs.GetArg("-conf", PIVX_CONF_FILENAME)); } catch (const std::exception& e) { diff --git a/src/util/system.cpp b/src/util/system.cpp index a262036a9e30..d3792e57d6a4 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -697,7 +697,10 @@ const fs::path& GetDataDir(bool fNetSpecific) if (fNetSpecific) path /= BaseParams().DataDir(); - fs::create_directories(path); + if (fs::create_directories(path)) { + // This is the first run, create wallets subdirectory too + fs::create_directories(path / "wallets"); + } return path; } diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index d2caf86fab8e..0f7e0fd06516 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -148,6 +148,15 @@ bool WalletVerify() return true; } + if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) { + if (fs::exists(fs::system_complete(gArgs.GetArg("-walletdir", "")))) { + return UIError(strprintf(_("Specified -walletdir \"%s\" is not a directory"), gArgs.GetArg("-walletdir", "").c_str())); + } + return UIError(strprintf(_("Specified -walletdir \"%s\" does not exist"), gArgs.GetArg("-walletdir", "").c_str())); + } + + LogPrintf("Using wallet directory %s\n", GetWalletDir().string()); + uiInterface.InitMessage(_("Verifying wallet(s)...")); // Keep track of each wallet absolute path to detect duplicates. diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index fece219f49e3..2daef700a006 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -18,6 +18,7 @@ #include "util/system.h" #include "utiltime.h" #include "wallet/wallet.h" +#include "wallet/walletutil.h" #include #include @@ -974,7 +975,7 @@ bool BackupWallet(const CWallet& wallet, const fs::path& strDest) // Copy wallet file fs::path pathDest(strDest); - fs::path pathSrc = GetDataDir() / strFile; + fs::path pathSrc = GetWalletDir() / strFile; if (is_directory(pathDest)) { if(!exists(pathDest)) create_directory(pathDest); pathDest /= strFile; diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py index dbe8f91a8c52..4c839e8e329c 100755 --- a/test/functional/wallet_backup.py +++ b/test/functional/wallet_backup.py @@ -98,9 +98,9 @@ def stop_three(self): self.stop_node(2) def erase_three(self): - os.remove(self.options.tmpdir + "/node0/regtest/wallet.dat") - os.remove(self.options.tmpdir + "/node1/regtest/wallet.dat") - os.remove(self.options.tmpdir + "/node2/regtest/wallet.dat") + os.remove(self.options.tmpdir + "/node0/regtest/wallets/wallet.dat") + os.remove(self.options.tmpdir + "/node1/regtest/wallets/wallet.dat") + os.remove(self.options.tmpdir + "/node2/regtest/wallets/wallet.dat") def run_test(self): self.log.info("Generating initial blockchain") @@ -162,9 +162,9 @@ def run_test(self): shutil.rmtree(self.options.tmpdir + "/node2/regtest/chainstate") # Restore wallets from backup - shutil.copyfile(tmpdir + "/node0/wallet.bak", tmpdir + "/node0/regtest/wallet.dat") - shutil.copyfile(tmpdir + "/node1/wallet.bak", tmpdir + "/node1/regtest/wallet.dat") - shutil.copyfile(tmpdir + "/node2/wallet.bak", tmpdir + "/node2/regtest/wallet.dat") + shutil.copyfile(tmpdir + "/node0/wallet.bak", tmpdir + "/node0/regtest/wallets/wallet.dat") + shutil.copyfile(tmpdir + "/node1/wallet.bak", tmpdir + "/node1/regtest/wallets/wallet.dat") + shutil.copyfile(tmpdir + "/node2/wallet.bak", tmpdir + "/node2/regtest/wallets/wallet.dat") self.log.info("Re-starting nodes") self.start_three() @@ -200,10 +200,10 @@ def run_test(self): # Backup to source wallet file must fail sourcePaths = [ - tmpdir + "/node0/regtest/wallet.dat", - tmpdir + "/node0/./regtest/wallet.dat", - tmpdir + "/node0/regtest/", - tmpdir + "/node0/regtest"] + tmpdir + "/node0/regtest/wallets/wallet.dat", + tmpdir + "/node0/./regtest/wallets/wallet.dat", + tmpdir + "/node0/regtest/wallets/", + tmpdir + "/node0/regtest/wallets"] for sourcePath in sourcePaths: assert_raises_rpc_error(-4, "backup failed", self.nodes[0].backupwallet, sourcePath) diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py index d52d8d996372..e0d295b951c0 100755 --- a/test/functional/wallet_hd.py +++ b/test/functional/wallet_hd.py @@ -131,7 +131,7 @@ def run_test(self): # otherwise node1 would auto-recover all funds in flag the keypool keys as used shutil.rmtree(os.path.join(self.nodes[1].datadir, "regtest", "blocks")) shutil.rmtree(os.path.join(self.nodes[1].datadir, "regtest", "chainstate")) - shutil.copyfile(os.path.join(self.nodes[1].datadir, "hd.bak"), os.path.join(self.nodes[1].datadir, "regtest", "wallet.dat")) + shutil.copyfile(os.path.join(self.nodes[1].datadir, "hd.bak"), os.path.join(self.nodes[1].datadir, "regtest", "wallets", "wallet.dat")) self.start_node(1) # Assert that derivation is deterministic @@ -154,7 +154,7 @@ def run_test(self): self.stop_node(1) shutil.rmtree(os.path.join(self.nodes[1].datadir, "regtest", "blocks")) shutil.rmtree(os.path.join(self.nodes[1].datadir, "regtest", "chainstate")) - shutil.copyfile(os.path.join(self.nodes[1].datadir, "hd.bak"), os.path.join(self.nodes[1].datadir, "regtest", "wallet.dat")) + shutil.copyfile(os.path.join(self.nodes[1].datadir, "hd.bak"), os.path.join(self.nodes[1].datadir, "regtest", "wallets", "wallet.dat")) self.start_and_connect_node1() # Wallet automatically scans blocks older than key on startup (but shielded addresses need to be regenerated) assert_equal(self.nodes[1].getbalance(), NUM_HD_ADDS + 1) @@ -213,7 +213,7 @@ def run_test(self): # Delete wallet and recover from first seed self.stop_node(1) - os.remove(os.path.join(self.nodes[1].datadir, "regtest", "wallet.dat")) + os.remove(os.path.join(self.nodes[1].datadir, "regtest", "wallets", "wallet.dat")) self.start_node(1) # Regenerate old shield addresses self.log.info("Restore from seed ...") diff --git a/test/functional/wallet_keypool_topup.py b/test/functional/wallet_keypool_topup.py index c2d301d01496..080799a0367a 100755 --- a/test/functional/wallet_keypool_topup.py +++ b/test/functional/wallet_keypool_topup.py @@ -36,7 +36,7 @@ def run_test(self): self.stop_node(1) - shutil.copyfile(self.tmpdir + "/node1/regtest/wallet.dat", self.tmpdir + "/wallet.bak") + shutil.copyfile(self.tmpdir + "/node1/regtest/wallets/wallet.dat", self.tmpdir + "/wallet.bak") self.start_node(1, self.extra_args[1]) connect_nodes(self.nodes[0], 1) @@ -59,7 +59,7 @@ def run_test(self): self.stop_node(1) - shutil.copyfile(self.tmpdir + "/wallet.bak", self.tmpdir + "/node1/regtest/wallet.dat") + shutil.copyfile(self.tmpdir + "/wallet.bak", self.tmpdir + "/node1/regtest/wallets/wallet.dat") self.log.info("Verify keypool is restored and balance is correct") diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index a97cacdec28a..ee3594bd1514 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -25,9 +25,8 @@ def set_test_params(self): def run_test(self): node = self.nodes[0] - # !TODO: backport bitcoin#12220 - #data_dir = lambda *p: os.path.join(node.datadir, 'regtest', *p) - #wallet_dir = lambda *p: data_dir('wallets', *p) + data_dir = lambda *p: os.path.join(node.datadir, 'regtest', *p) + wallet_dir = lambda *p: data_dir('wallets', *p) wallet = lambda name: node.get_wallet_rpc(name) assert_equal(set(node.listwallets()), {"w1", "w2", "w3", "w"}) @@ -35,7 +34,7 @@ def run_test(self): self.stop_nodes() # !TODO: backport bitcoin#12220 - #self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist') + self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist') #self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir()) #self.assert_start_raises_init_error(0, ['-walletdir=debug.log'], 'Error: Specified -walletdir "debug.log" is not a directory', cwd=data_dir()) @@ -43,22 +42,24 @@ def run_test(self): self.assert_start_raises_init_error(0, ['-wallet=w1', '-wallet=w1'], 'Error loading wallet w1. Duplicate -wallet filename specified.') # should not initialize if wallet file is a directory - os.mkdir(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w11')) - #os.mkdir(wallet_dir('w11')) + os.mkdir(wallet_dir('w11')) self.assert_start_raises_init_error(0, ['-wallet=w11'], 'Error loading wallet w11. -wallet filename must be a regular file.') #should not initialize if one wallet is a copy of another - #shutil.copyfile(wallet_dir('w2'), wallet_dir('w22')) # !TODO: backport bitcoin#11970 - shutil.copyfile(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w2'), - os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w22')) + shutil.copyfile(wallet_dir('w2'), wallet_dir('w22')) # !TODO: backport bitcoin#11970 self.assert_start_raises_init_error(0, ['-wallet=w2', '-wallet=w22'], 'duplicates fileid') # should not initialize if wallet file is a symlink - os.symlink(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w1'), - os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w12')) - # os.symlink(wallet_dir('w1'), wallet_dir('w12')) # !TODO: backport bitcoin#11970 + os.symlink(wallet_dir('w1'), wallet_dir('w12')) # !TODO: backport bitcoin#11970 self.assert_start_raises_init_error(0, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.') + # should not initialize if the specified walletdir does not exist + self.assert_start_raises_init_error(0, ['-walletdir=bad'], 'Error: Specified -walletdir "bad" does not exist') + # should not initialize if the specified walletdir is not a directory + not_a_dir = wallet_dir('notadir') + open(not_a_dir, 'a', encoding="utf8").close() + self.assert_start_raises_init_error(0, ['-walletdir='+not_a_dir], 'Error: Specified -walletdir "' + not_a_dir + '" is not a directory') + self.log.info("Do not allow -zapwallettxes with multiwallet") self.assert_start_raises_init_error(0, ['-zapwallettxes', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file") self.assert_start_raises_init_error(0, ['-zapwallettxes=1', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file") From 1dc22190bc1b6e48dd0f18462c5143dd0967c489 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Thu, 18 Jan 2018 13:15:00 -0500 Subject: [PATCH 07/32] Don't allow relative -walletdir paths Also warn if bitcoind is configured to use a relative -datadir path. Specifying paths relative to the current working directory in a daemon process can be dangerous, because files can fail to be located even if the configuration doesn't change, but the daemon is started up differently. Specifying a relative -datadir now adds a warning to the debug log. It would not be backwards-compatible to forbid relative -datadir paths entirely, and it could also be also inconvenient for command line testing. Specifying a relative -walletdir now results in a startup error. But since the -walletdir option is new in 0.16.0, there should be no compatibility issues. Another reason not to use working directory paths for -walletdir specifically is that the default -walletdir is a "wallets" subdirectory inside the datadir, so it could be surprising that setting -walletdir manually would choose a directory rooted in a completely different location. --- doc/release-notes.md | 33 ++++++++++++++++----------- src/init.cpp | 9 ++++++++ src/wallet/init.cpp | 12 ++++++---- src/wallet/walletutil.cpp | 2 +- test/functional/wallet_multiwallet.py | 5 ++-- 5 files changed, 40 insertions(+), 21 deletions(-) diff --git a/doc/release-notes.md b/doc/release-notes.md index 62d81590fc4b..6067946c925c 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -299,19 +299,26 @@ The `getwalletinfo` RPC method returns the name of the wallet used for the query Note that while multi-wallet is now fully supported, the RPC multi-wallet interface should be considered unstable for version 6.0.0, and there may backwards-incompatible changes in future versions. -Custom wallet directories ---------------------- -The ability to specify a directory other than the default data directory in which to store -wallets has been added. An existing directory can be specified using the `-walletdir=` -argument. Wallets loaded via `-wallet` arguments must be in this wallet directory. Care should be taken -when choosing a wallet directory location, as if it becomes unavailable during operation, -funds may be lost. - -Default wallet directory change --------------------------- -On new installations (if the data directory doesn't exist), wallets will now be stored in a -new `wallets/` subdirectory inside the data directory. If this `wallets/` subdirectory -doesn't exist (i.e. on existing nodes), the current datadir root is used instead, as it was. +Wallets directory configuration (`-walletdir`) +---------------------------------------------- + +PIVX Core now has more flexibility in where the wallets directory can be +located. Previously wallet database files were stored at the top level of the +PIVX data directory. The behavior is now: + +- For new installations (where the data directory doesn't already exist), + wallets will now be stored in a new `wallets/` subdirectory inside the data + directory by default. +- For existing nodes (where the data directory already exists), wallets will be + stored in the data directory root by default. If a `wallets/` subdirectory + already exists in the data directory root, then wallets will be stored in the + `wallets/` subdirectory by default. +- The location of the wallets directory can be overridden by specifying a + `-walletdir=` option where `` can be an absolute path to a + directory or directory symlink. + +Care should be taken when choosing the wallets directory location, as if it +becomes unavailable during operation, funds may be lost. Database cache memory increased -------------------------------- diff --git a/src/init.cpp b/src/init.cpp index 9a80c481892b..236e668cb47d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1272,6 +1272,15 @@ bool AppInitMain() LogPrintf("Using at most %i connections (%i file descriptors available)\n", nMaxConnections, nFD); std::ostringstream strErrors; + // Warn about relative -datadir path. + if (gArgs.IsArgSet("-datadir") && !fs::path(gArgs.GetArg("-datadir", "")).is_absolute()) { + LogPrintf("Warning: relative datadir option '%s' specified, which will be interpreted relative to the " + "current working directory '%s'. This is fragile, because if bitcoin is started in the future " + "from a different location, it will be unable to locate the current data files. There could " + "also be data loss if bitcoin is started while in a temporary directory.\n", + gArgs.GetArg("-datadir", ""), fs::current_path().string()); + } + InitSignatureCache(); LogPrintf("Using %u threads for script verification\n", nScriptCheckThreads); diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 0f7e0fd06516..4ce0ccf49279 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -148,11 +148,15 @@ bool WalletVerify() return true; } - if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) { - if (fs::exists(fs::system_complete(gArgs.GetArg("-walletdir", "")))) { - return UIError(strprintf(_("Specified -walletdir \"%s\" is not a directory"), gArgs.GetArg("-walletdir", "").c_str())); + if (gArgs.IsArgSet("-walletdir")) { + fs::path wallet_dir = gArgs.GetArg("-walletdir", ""); + if (!fs::exists(wallet_dir)) { + return UIError(strprintf(_("Specified -walletdir \"%s\" does not exist"), wallet_dir.string())); + } else if (!fs::is_directory(wallet_dir)) { + return UIError(strprintf(_("Specified -walletdir \"%s\" is not a directory"), wallet_dir.string())); + } else if (!wallet_dir.is_absolute()) { + return UIError(strprintf(_("Specified -walletdir \"%s\" is a relative path"), wallet_dir.string())); } - return UIError(strprintf(_("Specified -walletdir \"%s\" does not exist"), gArgs.GetArg("-walletdir", "").c_str())); } LogPrintf("Using wallet directory %s\n", GetWalletDir().string()); diff --git a/src/wallet/walletutil.cpp b/src/wallet/walletutil.cpp index 2f47d911640a..6bccc7182d98 100644 --- a/src/wallet/walletutil.cpp +++ b/src/wallet/walletutil.cpp @@ -10,7 +10,7 @@ fs::path GetWalletDir() fs::path path; if (gArgs.IsArgSet("-walletdir")) { - path = fs::system_complete(gArgs.GetArg("-walletdir", "")); + path = gArgs.GetArg("-walletdir", ""); if (!fs::is_directory(path)) { // If the path specified doesn't exist, we return the deliberately // invalid empty string. diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index ee3594bd1514..00dc3b993011 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -33,10 +33,9 @@ def run_test(self): self.stop_nodes() - # !TODO: backport bitcoin#12220 self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist') - #self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir()) - #self.assert_start_raises_init_error(0, ['-walletdir=debug.log'], 'Error: Specified -walletdir "debug.log" is not a directory', cwd=data_dir()) + self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir()) + self.assert_start_raises_init_error(0, ['-walletdir=debug.log'], 'Error: Specified -walletdir "debug.log" is not a directory', cwd=data_dir()) # should not initialize if there are duplicate wallets self.assert_start_raises_init_error(0, ['-wallet=w1', '-wallet=w1'], 'Error loading wallet w1. Duplicate -wallet filename specified.') From a238a8dfe621206624abafea1ce66297d79427df Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Fri, 15 Dec 2017 11:06:22 +1300 Subject: [PATCH 08/32] Add a lock to the wallet directory --- src/wallet/db.cpp | 66 +++++++++++++++++++++++++++++++++-------------- src/wallet/db.h | 2 +- 2 files changed, 47 insertions(+), 21 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index f0bc8e512396..04d4c5436473 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -20,6 +20,7 @@ #include #endif +#include #include @@ -55,6 +56,24 @@ void CheckUniqueFileid(const CDBEnv& env, const std::string& filename, Db& db) } } } + +bool LockEnvDirectory(const fs::path& env_path) +{ + // Make sure only a single PIVX process is using the wallet directory. + fs::path lock_file_path = env_path / ".lock"; + FILE* file = fsbridge::fopen(lock_file_path, "a"); // empty lock file; created if it doesn't exist. + if (file) fclose(file); + + try { + static boost::interprocess::file_lock lock(lock_file_path.string().c_str()); + if (!lock.try_lock()) { + return false; + } + } catch (const boost::interprocess::interprocess_exception& e) { + return error("Error obtaining lock on wallet directory %s: %s.", env_path.string(), e.what()); + } + return true; +} } // namespace // @@ -98,13 +117,17 @@ void CDBEnv::Close() EnvShutdown(); } -bool CDBEnv::Open(const fs::path& pathIn) +bool CDBEnv::Open(const fs::path& pathIn, bool retry) { if (fDbEnvInit) return true; boost::this_thread::interruption_point(); + if (!LockEnvDirectory(pathIn)) { + return false; + } + strPath = pathIn.string(); fs::path pathLogDir = pathIn / "database"; TryCreateDirectories(pathLogDir); @@ -137,7 +160,24 @@ bool CDBEnv::Open(const fs::path& pathIn) S_IRUSR | S_IWUSR); if (ret != 0) { dbenv->close(0); - return error("CDBEnv::Open : Error %d opening database environment: %s\n", ret, DbEnv::strerror(ret)); + LogPrintf("CDBEnv::Open: Error %d opening database environment: %s\n", ret, DbEnv::strerror(ret)); + if (retry) { + // try moving the database env out of the way + fs::path pathDatabaseBak = pathIn / strprintf("database.%d.bak", GetTime()); + try { + fs::rename(pathLogDir, pathDatabaseBak); + LogPrintf("Moved old %s to %s. Retrying.\n", pathLogDir.string(), pathDatabaseBak.string()); + } catch (const fs::filesystem_error&) { + // failure is ok (well, not really, but it's not worse than what we started with) + } + // try opening it again one more time + if (!Open(pathIn, false)) { + // if it still fails, it probably means we can't even create the database env + return false; + } + } else { + return false; + } } fDbEnvInit = true; @@ -266,25 +306,11 @@ bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walle if (walletFile != wallet_file_path.filename().string()) return UIError(strprintf(_("Wallet %s resides outside data directory %s"), walletFile, walletDir.string())); - if (!bitdb.Open(walletDir)) - { - // try moving the database env out of the way - fs::path pathDatabase = walletDir / "database"; - fs::path pathDatabaseBak = walletDir / strprintf("database.%d.bak", GetTime()); - try { - fs::rename(pathDatabase, pathDatabaseBak); - LogPrintf("Moved old %s to %s. Retrying.\n", pathDatabase.string(), pathDatabaseBak.string()); - } catch (const fs::filesystem_error&) { - // failure is ok (well, not really, but it's not worse than what we started with) - } - - // try again - if (!bitdb.Open(walletDir)) { - // if it still fails, it probably means we can't even create the database env - errorStr = strprintf(_("Error initializing wallet database environment %s!"), walletDir); - return false; - } + if (!bitdb.Open(walletDir, true)) { + errorStr = strprintf(_("Cannot obtain a lock on wallet directory %s. Another instance of bitcoin may be using it."), walletDir); + return false; } + return true; } diff --git a/src/wallet/db.h b/src/wallet/db.h index 168f6522f986..5254f1c41d6e 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -70,7 +70,7 @@ class CDBEnv typedef std::pair, std::vector > KeyValPair; bool Salvage(const std::string& strFile, bool fAggressive, std::vector& vResult); - bool Open(const fs::path& path); + bool Open(const fs::path& path, bool retry = 0); void Close(); void Flush(bool fShutdown); void CheckpointLSN(const std::string& strFile); From ddcfd4aaa13ab4e32604bac15dd330ac9aecc747 Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 15 Jun 2021 10:15:56 -0300 Subject: [PATCH 09/32] Enable test for wallet directory locking --- test/functional/wallet_multiwallet.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 00dc3b993011..dda940f08924 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -73,7 +73,7 @@ def run_test(self): self.assert_start_raises_init_error(0, ['-upgradewallet=1', '-wallet=w1', '-wallet=w2'], "Error: -upgradewallet is only allowed with a single wallet file") # !TODO: backport bitcoin#12220 - ''' + # if wallets/ doesn't exist, datadir should be the default wallet dir wallet_dir2 = data_dir('walletdir') os.rename(wallet_dir(), wallet_dir2) @@ -94,7 +94,7 @@ def run_test(self): os.mkdir(competing_wallet_dir) self.restart_node(0, ['-walletdir='+competing_wallet_dir]) self.assert_start_raises_init_error(1, ['-walletdir='+competing_wallet_dir], 'Error initializing wallet database environment') - ''' + self.restart_node(0, self.extra_args[0]) w1 = wallet("w1") From d8539bbdbb16c5107ecd7e01181027d89ccfdd19 Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Sun, 24 Dec 2017 12:45:33 +1300 Subject: [PATCH 10/32] Generalise walletdir lock error message for correctness --- src/wallet/db.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 04d4c5436473..de3e4e3c0cd0 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -124,11 +124,12 @@ bool CDBEnv::Open(const fs::path& pathIn, bool retry) boost::this_thread::interruption_point(); + strPath = pathIn.string(); if (!LockEnvDirectory(pathIn)) { + LogPrintf("Cannot obtain a lock on wallet directory %s. Another instance of PIVX may be using it.", strPath); return false; } - strPath = pathIn.string(); fs::path pathLogDir = pathIn / "database"; TryCreateDirectories(pathLogDir); fs::path pathErrorFile = pathIn / "db.log"; @@ -307,7 +308,7 @@ bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walle return UIError(strprintf(_("Wallet %s resides outside data directory %s"), walletFile, walletDir.string())); if (!bitdb.Open(walletDir, true)) { - errorStr = strprintf(_("Cannot obtain a lock on wallet directory %s. Another instance of bitcoin may be using it."), walletDir); + errorStr = strprintf(_("Error initializing wallet database environment %s!"), walletDir); return false; } From 6a0380ad2072abd9195eaf30a5f3edd87032cf4d Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Tue, 26 Dec 2017 19:18:39 +1300 Subject: [PATCH 11/32] Make .walletlock distinct from .lock --- src/wallet/db.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index de3e4e3c0cd0..a0ed2c4592aa 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -60,7 +60,7 @@ void CheckUniqueFileid(const CDBEnv& env, const std::string& filename, Db& db) bool LockEnvDirectory(const fs::path& env_path) { // Make sure only a single PIVX process is using the wallet directory. - fs::path lock_file_path = env_path / ".lock"; + fs::path lock_file_path = env_path / ".walletlock"; FILE* file = fsbridge::fopen(lock_file_path, "a"); // empty lock file; created if it doesn't exist. if (file) fclose(file); @@ -126,7 +126,7 @@ bool CDBEnv::Open(const fs::path& pathIn, bool retry) strPath = pathIn.string(); if (!LockEnvDirectory(pathIn)) { - LogPrintf("Cannot obtain a lock on wallet directory %s. Another instance of PIVX may be using it.", strPath); + LogPrintf("Cannot obtain a lock on wallet directory %s. Another instance of PIVX may be using it.\n", strPath); return false; } From 434ed7598cef245718fdf8a198e632a9452f40bf Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 15 Jun 2021 10:23:23 -0300 Subject: [PATCH 12/32] Abstract LockDirectory into system.cpp Adaptation of btc@2f3bd47d44634cfc0a4261e64af178407ce2869c --- src/init.cpp | 23 +++-------------------- src/util/system.cpp | 23 +++++++++++++++++++++++ src/util/system.h | 1 + src/wallet/db.cpp | 21 +-------------------- 4 files changed, 28 insertions(+), 40 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 236e668cb47d..93730bdb7624 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1199,27 +1199,10 @@ bool AppInitParameterInteraction() static bool LockDataDirectory(bool probeOnly) { - std::string strDataDir = GetDataDir().string(); - // Make sure only a single PIVX process is using the data directory. - fs::path pathLockFile = GetDataDir() / ".lock"; - FILE* file = fsbridge::fopen(pathLockFile, "a"); // empty lock file; created if it doesn't exist. - if (file) fclose(file); - - try { - static boost::interprocess::file_lock lock(pathLockFile.string().c_str()); - // Wait maximum 10 seconds if an old wallet is still running. Avoids lockup during restart - if (!lock.timed_lock(boost::get_system_time() + boost::posix_time::seconds(10))) { - return UIError(strprintf(_("Cannot obtain a lock on data directory %s. %s is probably already running."), - strDataDir, _(PACKAGE_NAME))); - } - if (probeOnly) { - lock.unlock(); - } - } catch (const boost::interprocess::interprocess_exception& e) { - return UIError( - strprintf(_("Cannot obtain a lock on data directory %s. %s is probably already running.") + " %s.", - strDataDir, _(PACKAGE_NAME), e.what())); + fs::path datadir = GetDataDir(); + if (!LockDirectory(datadir, ".lock", probeOnly)) { + return UIError(strprintf(_("Cannot obtain a lock on data directory %s. %s is probably already running."), datadir.string(), _(PACKAGE_NAME))); } return true; } diff --git a/src/util/system.cpp b/src/util/system.cpp index d3792e57d6a4..193ef2e676e8 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -77,6 +77,8 @@ #include #endif +#include + const char * const PIVX_CONF_FILENAME = "pivx.conf"; const char * const PIVX_PID_FILENAME = "pivx.pid"; const char * const PIVX_MASTERNODE_CONF_FILENAME = "masternode.conf"; @@ -103,6 +105,27 @@ bool CheckDiskSpace(const fs::path& dir, uint64_t additional_bytes) return free_bytes_available >= min_disk_space + additional_bytes; } +bool LockDirectory(const fs::path& directory, const std::string& lockfile_name, bool probe_only) +{ + fs::path pathLockFile = directory / lockfile_name; + FILE* file = fsbridge::fopen(pathLockFile, "a"); // empty lock file; created if it doesn't exist. + if (file) fclose(file); + + try { + static std::map locks; + boost::interprocess::file_lock& lock = locks.emplace(pathLockFile.string(), pathLockFile.string().c_str()).first->second; + if (!lock.try_lock()) { + return false; + } + if (probe_only) { + lock.unlock(); + } + } catch (const boost::interprocess::interprocess_exception& e) { + return error("Error while attempting to lock directory %s: %s", directory.string(), e.what()); + } + return true; +} + /** * Interpret a string argument as a boolean. * diff --git a/src/util/system.h b/src/util/system.h index 8837ee8784bf..3a8f5cf0404b 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -88,6 +88,7 @@ int RaiseFileDescriptorLimit(int nMinFD); void AllocateFileRange(FILE* file, unsigned int offset, unsigned int length); bool CheckDiskSpace(const fs::path& dir, uint64_t additional_bytes = 0); bool RenameOver(fs::path src, fs::path dest); +bool LockDirectory(const fs::path& directory, const std::string& lockfile_name, bool probe_only=false); bool TryCreateDirectories(const fs::path& p); fs::path GetDefaultDataDir(); // The blocks directory is always net specific. diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index a0ed2c4592aa..b00671b6652b 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -20,7 +20,6 @@ #include #endif -#include #include @@ -56,24 +55,6 @@ void CheckUniqueFileid(const CDBEnv& env, const std::string& filename, Db& db) } } } - -bool LockEnvDirectory(const fs::path& env_path) -{ - // Make sure only a single PIVX process is using the wallet directory. - fs::path lock_file_path = env_path / ".walletlock"; - FILE* file = fsbridge::fopen(lock_file_path, "a"); // empty lock file; created if it doesn't exist. - if (file) fclose(file); - - try { - static boost::interprocess::file_lock lock(lock_file_path.string().c_str()); - if (!lock.try_lock()) { - return false; - } - } catch (const boost::interprocess::interprocess_exception& e) { - return error("Error obtaining lock on wallet directory %s: %s.", env_path.string(), e.what()); - } - return true; -} } // namespace // @@ -125,7 +106,7 @@ bool CDBEnv::Open(const fs::path& pathIn, bool retry) boost::this_thread::interruption_point(); strPath = pathIn.string(); - if (!LockEnvDirectory(pathIn)) { + if (!LockDirectory(pathIn, ".walletlock")) { LogPrintf("Cannot obtain a lock on wallet directory %s. Another instance of PIVX may be using it.\n", strPath); return false; } From 8b8725d177cfadc3b408b3ae96a4561484fbd99d Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 15 Jun 2021 11:31:03 -0300 Subject: [PATCH 13/32] wallet_tests: Use dummy wallet instance instead of wallet pointer. --- src/wallet/test/wallet_tests.cpp | 161 ++++++++++++++++--------------- 1 file changed, 81 insertions(+), 80 deletions(-) diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 72a32716cf4b..722c761bbd54 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -35,9 +35,10 @@ typedef std::set > CoinSet; BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup) +static const CWallet testWallet; static std::vector vCoins; -static void add_coin(std::unique_ptr& pwallet, const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0) +static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0) { static int nextLockTime = 0; CMutableTransaction tx; @@ -49,7 +50,7 @@ static void add_coin(std::unique_ptr& pwallet, const CAmount& nValue, i // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() tx.vin.resize(1); } - std::unique_ptr wtx(new CWalletTx(pwallet.get(), MakeTransactionRef(std::move(tx)))); + std::unique_ptr wtx(new CWalletTx(&testWallet, MakeTransactionRef(std::move(tx)))); if (fIsFromMe) { wtx->m_amounts[CWalletTx::DEBIT].Set(ISMINE_SPENDABLE, 1); } @@ -75,7 +76,7 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) CoinSet setCoinsRet, setCoinsRet2; CAmount nValueRet; - LOCK(pwalletMain->cs_wallet); + LOCK(testWallet.cs_wallet); // test multiple times to allow for differences in the shuffle order for (int i = 0; i < RUN_TESTS; i++) @@ -83,148 +84,148 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) empty_wallet(); // with an empty wallet we can't even pay one cent - BOOST_CHECK(!pwalletMain->SelectCoinsMinConf( 1 * CENT, 1, 6, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, 1, 6, vCoins, setCoinsRet, nValueRet)); - add_coin(pwalletMain, 1*CENT, 4); // add a new 1 cent coin + add_coin(1*CENT, 4); // add a new 1 cent coin // with a new 1 cent coin, we still can't find a mature 1 cent - BOOST_CHECK(!pwalletMain->SelectCoinsMinConf( 1 * CENT, 1, 6, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, 1, 6, vCoins, setCoinsRet, nValueRet)); // but we can find a new 1 cent - BOOST_CHECK(pwalletMain->SelectCoinsMinConf( 1 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf( 1 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); - add_coin(pwalletMain, 2*CENT); // add a mature 2 cent coin + add_coin(2*CENT); // add a mature 2 cent coin // we can't make 3 cents of mature coins - BOOST_CHECK(!pwalletMain->SelectCoinsMinConf( 3 * CENT, 1, 6, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 3 * CENT, 1, 6, vCoins, setCoinsRet, nValueRet)); // we can make 3 cents of new coins - BOOST_CHECK(pwalletMain->SelectCoinsMinConf( 3 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf( 3 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); - add_coin(pwalletMain, 5*CENT); // add a mature 5 cent coin, - add_coin(pwalletMain, 10*CENT, 3, true); // a new 10 cent coin sent from one of our own addresses - add_coin(pwalletMain, 20*CENT); // and a mature 20 cent coin + add_coin(5*CENT); // add a mature 5 cent coin, + add_coin(10*CENT, 3, true); // a new 10 cent coin sent from one of our own addresses + add_coin(20*CENT); // and a mature 20 cent coin // now we have new: 1+10=11 (of which 10 was self-sent), and mature: 2+5+20=27. total = 38 // we can't make 38 cents only if we disallow new coins: - BOOST_CHECK(!pwalletMain->SelectCoinsMinConf(38 * CENT, 1, 6, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, 1, 6, vCoins, setCoinsRet, nValueRet)); // we can't even make 37 cents if we don't allow new coins even if they're from us - BOOST_CHECK(!pwalletMain->SelectCoinsMinConf(38 * CENT, 6, 6, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, 6, 6, vCoins, setCoinsRet, nValueRet)); // but we can make 37 cents if we accept new coins from ourself - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(37 * CENT, 1, 6, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(37 * CENT, 1, 6, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); // and we can make 38 cents if we accept all new coins - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(38 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(38 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 38 * CENT); // try making 34 cents from 1,2,5,10,20 - we can't do it exactly - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(34 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(34 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_GT(nValueRet, 34 * CENT); // but should get more than 34 cents BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // the best should be 20+10+5. it's incredibly unlikely the 1 or 2 got included (but possible) // when we try making 7 cents, the smaller coins (1,2,5) are enough. We should see just 2+5 - BOOST_CHECK(pwalletMain->SelectCoinsMinConf( 7 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf( 7 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 7 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // when we try making 8 cents, the smaller coins (1,2,5) are exactly enough. - BOOST_CHECK(pwalletMain->SelectCoinsMinConf( 8 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf( 8 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK(nValueRet == 8 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // when we try making 9 cents, no subset of smaller coins is enough, and we get the next bigger coin (10) - BOOST_CHECK(pwalletMain->SelectCoinsMinConf( 9 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf( 9 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin empty_wallet(); - add_coin(pwalletMain, 6*CENT); - add_coin(pwalletMain, 7*CENT); - add_coin(pwalletMain, 8*CENT); - add_coin(pwalletMain, 20*CENT); - add_coin(pwalletMain, 30*CENT); // now we have 6+7+8+20+30 = 71 cents total + add_coin(6*CENT); + add_coin(7*CENT); + add_coin(8*CENT); + add_coin(20*CENT); + add_coin(30*CENT); // now we have 6+7+8+20+30 = 71 cents total // check that we have 71 and not 72 - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(71 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK(!pwalletMain->SelectCoinsMinConf(72 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(71 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); // now try making 16 cents. the best smaller coins can do is 6+7+8 = 21; not as good at the next biggest coin, 20 - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(16 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(16 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 20 * CENT); // we should get 20 in one coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - add_coin(pwalletMain, 5*CENT); // now we have 5+6+7+8+20+30 = 75 cents total + add_coin(5*CENT); // now we have 5+6+7+8+20+30 = 75 cents total // now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, better than the next biggest coin, 20 - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(16 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(16 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 3 coins BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); - add_coin(pwalletMain, 18*CENT); // now we have 5+6+7+8+18+20+30 + add_coin(18*CENT); // now we have 5+6+7+8+18+20+30 // and now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, the same as the next biggest coin, 18 - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(16 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(16 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // because in the event of a tie, the biggest coin wins // now try making 11 cents. we should get 5+6 - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(11 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(11 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // check that the smallest bigger coin is used - add_coin(pwalletMain, 1*COIN); - add_coin(pwalletMain, 2*COIN); - add_coin(pwalletMain, 3*COIN); - add_coin(pwalletMain, 4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(95 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + add_coin(1*COIN); + add_coin(2*COIN); + add_coin(3*COIN); + add_coin(4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents + BOOST_CHECK(testWallet.SelectCoinsMinConf(95 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * COIN); // we should get 1 BTC in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(195 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(195 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // empty the wallet and start again, now with fractions of a cent, to test sub-cent change avoidance empty_wallet(); - add_coin(pwalletMain, 0.1*CENT); - add_coin(pwalletMain, 0.2*CENT); - add_coin(pwalletMain, 0.3*CENT); - add_coin(pwalletMain, 0.4*CENT); - add_coin(pwalletMain, 0.5*CENT); + add_coin(0.1*CENT); + add_coin(0.2*CENT); + add_coin(0.3*CENT); + add_coin(0.4*CENT); + add_coin(0.5*CENT); // try making 1 cent from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 = 1.5 cents // we'll get sub-cent change whatever happens, so can expect 1.0 exactly - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(1 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(1 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); // but if we add a bigger coin, making it possible to avoid sub-cent change, things change: - add_coin(pwalletMain, 1111*CENT); + add_coin(1111*CENT); // try making 1 cent from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5 cents - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(1 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(1 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); // we should get the exact amount // if we add more sub-cent coins: - add_coin(pwalletMain, 0.6*CENT); - add_coin(pwalletMain, 0.7*CENT); + add_coin(0.6*CENT); + add_coin(0.7*CENT); // and try again to make 1.0 cents, we can still make 1.0 cents - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(1 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(1 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); // we should get the exact amount // run the 'mtgox' test (see http://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) // they tried to consolidate 10 50k coins into one 500k coin, and ended up with 50k in change empty_wallet(); for (int j = 0; j < 20; j++) - add_coin(pwalletMain, 50000 * COIN); + add_coin(50000 * COIN); - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(500000 * COIN, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(500000 * COIN, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins @@ -233,37 +234,37 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) // sometimes it will fail, and so we use the next biggest coin: empty_wallet(); - add_coin(pwalletMain, 0.5 * CENT); - add_coin(pwalletMain, 0.6 * CENT); - add_coin(pwalletMain, 0.7 * CENT); - add_coin(pwalletMain, 1111 * CENT); - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(1 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + add_coin(0.5 * CENT); + add_coin(0.6 * CENT); + add_coin(0.7 * CENT); + add_coin(1111 * CENT); + BOOST_CHECK(testWallet.SelectCoinsMinConf(1 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1111 * CENT); // we get the bigger coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) empty_wallet(); - add_coin(pwalletMain, 0.4 * CENT); - add_coin(pwalletMain, 0.6 * CENT); - add_coin(pwalletMain, 0.8 * CENT); - add_coin(pwalletMain, 1111 * CENT); - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(1 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); + add_coin(0.4 * CENT); + add_coin(0.6 * CENT); + add_coin(0.8 * CENT); + add_coin(1111 * CENT); + BOOST_CHECK(testWallet.SelectCoinsMinConf(1 * CENT, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // in two coins 0.4+0.6 // test avoiding sub-cent change empty_wallet(); - add_coin(pwalletMain, 0.0005 * COIN); - add_coin(pwalletMain, 0.01 * COIN); - add_coin(pwalletMain, 1 * COIN); + add_coin(0.0005 * COIN); + add_coin(0.01 * COIN); + add_coin(1 * COIN); // trying to make 1.0001 from these three coins - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(1.0001 * COIN, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(1.0001 * COIN, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1.0105 * COIN); // we should get all coins BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // but if we try to make 0.999, we should take the bigger of the two small coins to avoid sub-cent change - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(0.999 * COIN, 1, 1, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(0.999 * COIN, 1, 1, vCoins, setCoinsRet, nValueRet)); BOOST_CHECK_EQUAL(nValueRet, 1.01 * COIN); // we should get 1 + 0.01 BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); @@ -271,12 +272,12 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) { empty_wallet(); for (int i2 = 0; i2 < 100; i2++) - add_coin(pwalletMain, COIN); + add_coin(COIN); // picking 50 from 100 coins doesn't depend on the shuffle, // but does depend on randomness in the stochastic approximation code - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(50 * COIN, 1, 6, vCoins, setCoinsRet , nValueRet)); - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(50 * COIN, 1, 6, vCoins, setCoinsRet2, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, 1, 6, vCoins, setCoinsRet , nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, 1, 6, vCoins, setCoinsRet2, nValueRet)); BOOST_CHECK(!equal_sets(setCoinsRet, setCoinsRet2)); int fails = 0; @@ -284,8 +285,8 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) { // selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time // run the test RANDOM_REPEATS times and only complain if all of them fail - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(COIN, 1, 6, vCoins, setCoinsRet , nValueRet)); - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(COIN, 1, 6, vCoins, setCoinsRet2, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, 1, 6, vCoins, setCoinsRet , nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, 1, 6, vCoins, setCoinsRet2, nValueRet)); if (equal_sets(setCoinsRet, setCoinsRet2)) fails++; } @@ -294,19 +295,19 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) // add 75 cents in small change. not enough to make 90 cents, // then try making 90 cents. there are multiple competing "smallest bigger" coins, // one of which should be picked at random - add_coin(pwalletMain, 5*CENT); - add_coin(pwalletMain, 10*CENT); - add_coin(pwalletMain, 15*CENT); - add_coin(pwalletMain, 20*CENT); - add_coin(pwalletMain, 25*CENT); + add_coin(5 * CENT); + add_coin(10 * CENT); + add_coin(15 * CENT); + add_coin(20 * CENT); + add_coin(25 * CENT); fails = 0; for (int j = 0; j < RANDOM_REPEATS; j++) { // selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time // run the test RANDOM_REPEATS times and only complain if all of them fail - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(90*CENT, 1, 6, vCoins, setCoinsRet , nValueRet)); - BOOST_CHECK(pwalletMain->SelectCoinsMinConf(90*CENT, 1, 6, vCoins, setCoinsRet2, nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, 1, 6, vCoins, setCoinsRet , nValueRet)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, 1, 6, vCoins, setCoinsRet2, nValueRet)); if (equal_sets(setCoinsRet, setCoinsRet2)) fails++; } From d36185a137df2ff95ab2f8918d8a259e208d1382 Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 15 Jun 2021 12:15:24 -0300 Subject: [PATCH 14/32] wallet: unify backup creation process. Plus remove '-custombackupthreshold' and '-backuppath' options as they are not longer useful for multi-wallets. If we really want to implement then in the future, then better to code them properly. --- src/qt/forms/rpcconsole.ui | 137 +++++++------------------------------ src/qt/rpcconsole.cpp | 14 ---- src/qt/walletmodel.cpp | 4 +- src/wallet/init.cpp | 2 - src/wallet/rpcwallet.cpp | 2 +- src/wallet/wallet.cpp | 5 ++ src/wallet/wallet.h | 7 +- src/wallet/walletdb.cpp | 80 ---------------------- src/wallet/walletdb.h | 1 - 9 files changed, 38 insertions(+), 214 deletions(-) diff --git a/src/qt/forms/rpcconsole.ui b/src/qt/forms/rpcconsole.ui index e520dc500603..6ea75dba4d39 100644 --- a/src/qt/forms/rpcconsole.ui +++ b/src/qt/forms/rpcconsole.ui @@ -7,7 +7,7 @@ 0 0 769 - 496 + 498 @@ -17,7 +17,7 @@ - 3 + 4 @@ -425,7 +425,7 @@ - + :/icons/remove:/icons/remove @@ -1299,93 +1299,6 @@ - - - false - - - Custom Backup Path: - - - - - - - IBeamCursor - - - false - - - N/A - - - Qt::PlainText - - - Qt::LinksAccessibleByMouse|Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse - - - - - - - false - - - Custom zPIV Backup Path: - - - - - - - IBeamCursor - - - false - - - N/A - - - Qt::PlainText - - - Qt::LinksAccessibleByMouse|Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse - - - - - - - false - - - Custom Backups Threshold: - - - - - - - IBeamCursor - - - false - - - N/A - - - Qt::PlainText - - - Qt::LinksAccessibleByMouse|Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse - - - - Qt::Vertical @@ -1398,7 +1311,7 @@ - + @@ -1411,14 +1324,14 @@ - + -salvagewallet: - + Attempt to recover private keys from a corrupt wallet.dat. @@ -1428,7 +1341,7 @@ - + @@ -1441,14 +1354,14 @@ - + -rescan: - + Rescan the block chain for missing wallet transactions. @@ -1458,7 +1371,7 @@ - + @@ -1471,14 +1384,14 @@ - + -zapwallettxes=1: - + Recover transactions from blockchain (keep meta-data, e.g. account owner). @@ -1488,7 +1401,7 @@ - + @@ -1501,14 +1414,14 @@ - + -zapwallettxes=2: - + Recover transactions from blockchain (drop meta-data). @@ -1518,7 +1431,7 @@ - + @@ -1531,14 +1444,14 @@ - + -upgradewallet: - + Upgrade wallet to latest format on startup. (Note: this is NOT an update of the wallet itself!) @@ -1548,7 +1461,7 @@ - + @@ -1561,14 +1474,14 @@ - + -reindex: - + Rebuild block chain index from current blk000??.dat files. @@ -1578,7 +1491,7 @@ - + @@ -1591,14 +1504,14 @@ - + -resync: - + Deletes all local blockchain folders so the wallet synchronizes from scratch. @@ -1608,7 +1521,7 @@ - + Qt::Vertical diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 10da3b125dc4..a6057e6189e2 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -93,20 +93,6 @@ RPCConsole::RPCConsole(QWidget* parent) : QDialog(parent, Qt::WindowSystemMenuHi // set library version labels #ifdef ENABLE_WALLET - std::string strPathCustom = gArgs.GetArg("-backuppath", ""); - int nCustomBackupThreshold = gArgs.GetArg("-custombackupthreshold", DEFAULT_CUSTOMBACKUPTHRESHOLD); - - if(!strPathCustom.empty()) { - ui->wallet_custombackuppath->setText(QString::fromStdString(strPathCustom)); - ui->wallet_custombackuppath_label->show(); - ui->wallet_custombackuppath->show(); - if (nCustomBackupThreshold > 0) { - ui->wallet_custombackupthreshold->setText(QString::fromStdString(std::to_string(nCustomBackupThreshold))); - ui->wallet_custombackupthreshold_label->setVisible(true); - ui->wallet_custombackupthreshold->setVisible(true); - } - } - ui->berkeleyDBVersion->setText(DbEnv::version(0, 0, 0)); ui->wallet_path->setText(QString::fromStdString(GetDataDir().string() + QDir::separator().toLatin1() + gArgs.GetArg("-wallet", DEFAULT_WALLET_DAT))); #else diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index d5edb32f6f6d..034cfc89befd 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -730,8 +730,8 @@ bool WalletModel::changePassphrase(const SecureString& oldPass, const SecureStri bool WalletModel::backupWallet(const QString& filename) { - //attempt regular backup - if (!BackupWallet(*wallet, filename.toLocal8Bit().data())) { + // Attempt regular backup + if (!wallet->BackupWallet(filename.toLocal8Bit().data())) { return error("ERROR: Failed to backup wallet!"); } diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 4ce0ccf49279..d9c509e8faaf 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -17,9 +17,7 @@ std::string GetWalletHelpString(bool showDebug) { std::string strUsage = HelpMessageGroup(_("Wallet options:")); - strUsage += HelpMessageOpt("-backuppath=", _("Specify custom backup path to add a copy of any wallet backup. If set as dir, every backup generates a timestamped file. If set as file, will rewrite to that file every backup.")); strUsage += HelpMessageOpt("-createwalletbackups=", strprintf(_("Number of automatic wallet backups (default: %d)"), DEFAULT_CREATEWALLETBACKUPS)); - strUsage += HelpMessageOpt("-custombackupthreshold=", strprintf(_("Number of custom location backups to retain (default: %d)"), DEFAULT_CUSTOMBACKUPTHRESHOLD)); strUsage += HelpMessageOpt("-disablewallet", strprintf(_("Do not load the wallet and disable wallet RPC calls (default: %u)"), DEFAULT_DISABLE_WALLET)); strUsage += HelpMessageOpt("-keypool=", strprintf(_("Set key pool size to (default: %u)"), DEFAULT_KEYPOOL_SIZE)); strUsage += HelpMessageOpt("-legacywallet", _("On first run, create a legacy wallet instead of a HD wallet")); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 0f100f2f99d5..ab435f2fe863 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3373,7 +3373,7 @@ UniValue backupwallet(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); std::string strDest = request.params[0].get_str(); - if (!BackupWallet(*pwallet, strDest)) + if (!pwallet->BackupWallet(strDest)) throw JSONRPCError(RPC_WALLET_ERROR, "Error: Wallet backup failed!"); return NullUniValue; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9fa208e57c74..7fa8ba8bc0f9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4307,6 +4307,11 @@ void CWallet::postInitProcess(CScheduler& scheduler) } } +bool CWallet::BackupWallet(const std::string& strDest) +{ + return dbw->Backup(strDest); +} + CKeyPool::CKeyPool() { nTime = GetTime(); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 4a0575740f05..ff608b813410 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -65,8 +65,6 @@ static const CAmount nHighTransactionFeeWarning = 0.1 * COIN; static const CAmount DEFAULT_TRANSACTION_MAXFEE = 1 * COIN; //! -maxtxfee will warn if called with a higher fee than this amount (in satoshis) static const CAmount nHighTransactionMaxFeeWarning = 100 * nHighTransactionFeeWarning; -//! -custombackupthreshold default -static const int DEFAULT_CUSTOMBACKUPTHRESHOLD = 1; //! -minstakesplit default static const CAmount DEFAULT_MIN_STAKE_SPLIT_THRESHOLD = 100 * COIN; //! Default for -spendzeroconfchange @@ -1178,6 +1176,11 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface */ void postInitProcess(CScheduler& scheduler); + /** + * Creates a wallet backup in strDest path + */ + bool BackupWallet(const std::string& strDest); + /** * Blocks until the wallet state is up-to-date to /at least/ the current * chain at the time this function is entered diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 2daef700a006..d150319b177f 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -834,28 +834,6 @@ void NotifyBacked(const CWallet& wallet, bool fSuccess, std::string strMessage) wallet.NotifyWalletBacked(fSuccess, strMessage); } -// Returns first the pathCustom, second the pathWithFile. -std::pair GetBackupPath(const CWallet& wallet) -{ - fs::path pathCustom; - fs::path pathWithFile = gArgs.GetArg("-backuppath", ""); - if(!pathWithFile.empty()) { - if(!pathWithFile.has_extension()) { - pathCustom = pathWithFile; - pathWithFile /= wallet.GetUniqueWalletBackupName(); - } else { - pathCustom = pathWithFile.parent_path(); - } - try { - fs::create_directories(pathCustom); - } catch (const fs::filesystem_error& e) { - NotifyBacked(wallet, false, strprintf("%s\n", e.what())); - pathCustom = ""; - } - } - return {pathCustom, pathWithFile}; -} - typedef std::multimap folder_set_t; static folder_set_t buildBackupsMapSortedByLastWrite(const std::string& strWalletFile, const fs::path& backupsDir) { @@ -936,64 +914,6 @@ bool AutoBackupWallet(const CWallet& wallet, std::string& strBackupWarning, std: return cleanWalletBackups(folder_set, nWalletBackups, strBackupWarning); } -void MultiBackup(const CWallet& wallet, fs::path pathCustom, fs::path pathWithFile, const fs::path& pathSrc) -{ - int nThreshold = gArgs.GetArg("-custombackupthreshold", DEFAULT_CUSTOMBACKUPTHRESHOLD); - if (nThreshold > 0) { - std::string strBackupWarning; - pathCustom.make_preferred(); - folder_set_t folderSet = buildBackupsMapSortedByLastWrite(wallet.GetDBHandle().GetName(), pathCustom); - if (!cleanWalletBackups(folderSet, nThreshold, strBackupWarning)) { - NotifyBacked(wallet, false, strBackupWarning); - } - - // TODO: add seconds to avoid naming conflicts - for (const auto& entry : folderSet) { - if(entry.second == pathWithFile) { - pathWithFile += "(1)"; - } - } - } - AttemptBackupWallet(&wallet, pathSrc.string(), pathWithFile.string()); -} - -bool BackupWallet(const CWallet& wallet, const fs::path& strDest) -{ - const auto& pathsPair = GetBackupPath(wallet); - fs::path pathCustom = pathsPair.first; - fs::path pathWithFile = pathsPair.second; - - std::string strFile = wallet.GetDBHandle().GetName(); - 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 pathDest(strDest); - fs::path pathSrc = GetWalletDir() / strFile; - if (is_directory(pathDest)) { - if(!exists(pathDest)) create_directory(pathDest); - pathDest /= strFile; - } - bool defaultPath = AttemptBackupWallet(&wallet, pathSrc.string(), pathDest.string()); - - if(defaultPath && !pathCustom.empty()) { - MultiBackup(wallet, pathCustom, pathWithFile, pathSrc); - } - - return defaultPath; - } - } - MilliSleep(100); - } - return false; -} - bool AttemptBackupWallet(const CWallet* wallet, const fs::path& pathSrc, const fs::path& pathDest) { bool retStatus; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index c0803404ff3a..6da18d60b6a1 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -232,7 +232,6 @@ class CWalletDB }; void NotifyBacked(const CWallet& wallet, bool fSuccess, std::string strMessage); -bool BackupWallet(const CWallet& wallet, const fs::path& strDest); // If wallet is null, the NotifyBacked signal will not be broadcasted. // todo: move NotifyBacked() signal to the caller side and/or decouple it from here in another function bool AttemptBackupWallet(const CWallet* wallet, const fs::path& pathSrc, const fs::path& pathDest); From 9b2dae14f59cf4218b012b2f69d013c942ea9134 Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 15 Jun 2021 16:16:09 -0300 Subject: [PATCH 15/32] Allow wallet files in multiple directories Coming of btc@d8a99f65e53019becdd8d2631396012bafb29741 with further adaptations. Remove requirement that two wallet files can only be opened at the same time if they are contained in the same directory. This change consists of updates to function signatures (updating functions to take fs::path arguments, instead of combinations of strings, fs::path, and CDBEnv / CWalletDBWrapper arguments). --- src/test/librust/sapling_rpc_wallet_tests.cpp | 118 ++++++++--------- src/test/librust/sapling_wallet_tests.cpp | 28 ++-- src/test/librust/wallet_zkeys_tests.cpp | 32 +++-- src/test/miner_tests.cpp | 24 ++-- src/test/script_P2CS_tests.cpp | 2 +- src/test/validation_tests.cpp | 2 +- src/wallet/db.cpp | 120 +++++++++++++----- src/wallet/db.h | 55 +++++--- src/wallet/init.cpp | 10 +- ...sapling_transactions_validations_tests.cpp | 8 +- .../test/wallet_shielded_balances_tests.cpp | 10 +- src/wallet/test/wallet_test_fixture.cpp | 16 +-- src/wallet/test/wallet_test_fixture.h | 2 +- src/wallet/test/wallet_tests.cpp | 17 ++- src/wallet/wallet.cpp | 25 ++-- src/wallet/wallet.h | 25 ++-- src/wallet/walletdb.cpp | 22 ++-- src/wallet/walletdb.h | 8 +- test/functional/feature_config_args.py | 2 +- .../test_framework/test_framework.py | 2 +- test/functional/wallet_multiwallet.py | 4 +- test/functional/wallet_reorgsrestore.py | 2 +- 22 files changed, 295 insertions(+), 239 deletions(-) diff --git a/src/test/librust/sapling_rpc_wallet_tests.cpp b/src/test/librust/sapling_rpc_wallet_tests.cpp index 6f40f8f95b45..05cb6178a4b9 100644 --- a/src/test/librust/sapling_rpc_wallet_tests.cpp +++ b/src/test/librust/sapling_rpc_wallet_tests.cpp @@ -56,7 +56,7 @@ BOOST_FIXTURE_TEST_SUITE(sapling_rpc_wallet_tests, WalletTestingSetup) BOOST_AUTO_TEST_CASE(rpc_wallet_sapling_validateaddress) { SelectParams(CBaseChainParams::MAIN); - vpwallets.insert(vpwallets.begin(), pwalletMain.get()); + vpwallets.insert(vpwallets.begin(), &m_wallet); UniValue retValue; @@ -66,7 +66,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_sapling_validateaddress) // Wallet should be empty: std::set addrs; - pwalletMain->GetSaplingPaymentAddresses(addrs); + m_wallet.GetSaplingPaymentAddresses(addrs); BOOST_CHECK(addrs.size()==0); // This Sapling address is not valid, it belongs to another network @@ -91,11 +91,11 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_sapling_validateaddress) BOOST_AUTO_TEST_CASE(rpc_wallet_getbalance) { { - LOCK(pwalletMain->cs_wallet); - pwalletMain->SetMinVersion(FEATURE_SAPLING); - pwalletMain->SetupSPKM(false); + LOCK(m_wallet.cs_wallet); + m_wallet.SetMinVersion(FEATURE_SAPLING); + m_wallet.SetupSPKM(false); } - vpwallets.insert(vpwallets.begin(), pwalletMain.get()); + vpwallets.insert(vpwallets.begin(), &m_wallet); BOOST_CHECK_THROW(CallRPC("getshieldbalance too many args"), std::runtime_error); BOOST_CHECK_THROW(CallRPC("getshieldbalance invalidaddress"), std::runtime_error); @@ -122,11 +122,11 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_getbalance) BOOST_AUTO_TEST_CASE(rpc_wallet_sapling_importkey_paymentaddress) { { - LOCK(pwalletMain->cs_wallet); - pwalletMain->SetMinVersion(FEATURE_SAPLING); - pwalletMain->SetupSPKM(false); + LOCK(m_wallet.cs_wallet); + m_wallet.SetMinVersion(FEATURE_SAPLING); + m_wallet.SetupSPKM(false); } - vpwallets.insert(vpwallets.begin(), pwalletMain.get()); + vpwallets.insert(vpwallets.begin(), &m_wallet); auto testAddress = [](const std::string& key) { UniValue ret; @@ -153,11 +153,11 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_sapling_importkey_paymentaddress) BOOST_AUTO_TEST_CASE(rpc_wallet_sapling_importexport) { { - LOCK(pwalletMain->cs_wallet); - pwalletMain->SetMinVersion(FEATURE_SAPLING); - pwalletMain->SetupSPKM(false); + LOCK(m_wallet.cs_wallet); + m_wallet.SetMinVersion(FEATURE_SAPLING); + m_wallet.SetupSPKM(false); } - vpwallets.insert(vpwallets.begin(), pwalletMain.get()); + vpwallets.insert(vpwallets.begin(), &m_wallet); UniValue retValue; int n1 = 1000; // number of times to import/export @@ -181,7 +181,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_sapling_importexport) // wallet should currently be empty std::set saplingAddrs; - pwalletMain->GetSaplingPaymentAddresses(saplingAddrs); + m_wallet.GetSaplingPaymentAddresses(saplingAddrs); BOOST_CHECK(saplingAddrs.empty()); // verify import and export key @@ -209,13 +209,13 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_sapling_importexport) // Make new addresses for the set for (int i=0; iGenerateNewSaplingZKey())); + myaddrs.insert(KeyIO::EncodePaymentAddress(m_wallet.GenerateNewSaplingZKey())); } // Verify number of addresses stored in wallet is n1+n2 int numAddrs = myaddrs.size(); BOOST_CHECK(numAddrs == n1 + n2); - pwalletMain->GetSaplingPaymentAddresses(saplingAddrs); + m_wallet.GetSaplingPaymentAddresses(saplingAddrs); BOOST_CHECK((int) saplingAddrs.size() == numAddrs); // Ask wallet to list addresses @@ -237,27 +237,27 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_sapling_importexport) } // Check if address is of given type and spendable from our wallet. -void CheckHaveAddr(std::unique_ptr& pwallet, const libzcash::PaymentAddress& addr) +void CheckHaveAddr(CWallet& pwallet, const libzcash::PaymentAddress& addr) { BOOST_CHECK(IsValidPaymentAddress(addr)); auto addr_of_type = boost::get(&addr); BOOST_ASSERT(addr_of_type != nullptr); - BOOST_CHECK(pwallet->HaveSpendingKeyForPaymentAddress(*addr_of_type)); + BOOST_CHECK(pwallet.HaveSpendingKeyForPaymentAddress(*addr_of_type)); } BOOST_AUTO_TEST_CASE(rpc_wallet_getnewshieldaddress) { { - LOCK(pwalletMain->cs_wallet); - pwalletMain->SetMinVersion(FEATURE_SAPLING); - pwalletMain->SetupSPKM(false); + LOCK(m_wallet.cs_wallet); + m_wallet.SetMinVersion(FEATURE_SAPLING); + m_wallet.SetupSPKM(false); } - vpwallets.insert(vpwallets.begin(), pwalletMain.get()); + vpwallets.insert(vpwallets.begin(), &m_wallet); // No parameter defaults to sapling address UniValue addr = CallRPC("getnewshieldaddress"); - CheckHaveAddr(pwalletMain, KeyIO::DecodePaymentAddress(addr.get_str())); + CheckHaveAddr(m_wallet, KeyIO::DecodePaymentAddress(addr.get_str())); // Too many arguments will throw with the help BOOST_CHECK_THROW(CallRPC("getnewshieldaddress many args"), std::runtime_error); @@ -267,11 +267,11 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_getnewshieldaddress) BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_parameters) { { - LOCK(pwalletMain->cs_wallet); - pwalletMain->SetMinVersion(FEATURE_SAPLING); - pwalletMain->SetupSPKM(false); + LOCK(m_wallet.cs_wallet); + m_wallet.SetMinVersion(FEATURE_SAPLING); + m_wallet.SetupSPKM(false); } - vpwallets.insert(vpwallets.begin(), pwalletMain.get()); + vpwallets.insert(vpwallets.begin(), &m_wallet); BOOST_CHECK_THROW(CallRPC("shieldsendmany"), std::runtime_error); BOOST_CHECK_THROW(CallRPC("shieldsendmany toofewargs"), std::runtime_error); @@ -320,7 +320,7 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_parameters) std::vector v (2 * (ZC_MEMO_SIZE+1)); // x2 for hexadecimal string format std::fill(v.begin(),v.end(), 'A'); std::string badmemo(v.begin(), v.end()); - auto pa = pwalletMain->GenerateNewSaplingZKey(); + auto pa = m_wallet.GenerateNewSaplingZKey(); std::string zaddr1 = KeyIO::EncodePaymentAddress(pa); BOOST_CHECK_THROW(CallRPC(std::string("shieldsendmany DMKU6mc52un1MThGCsnNwAtEvncaTdAuaZ ") + "[{\"address\":\"" + zaddr1 + "\", \"amount\":123.456}]"), std::runtime_error); @@ -332,11 +332,11 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_parameters) BOOST_AUTO_TEST_CASE(saplingOperationTests) { { - LOCK2(cs_main, pwalletMain->cs_wallet); - pwalletMain->SetupSPKM(false); + LOCK2(cs_main, m_wallet.cs_wallet); + m_wallet.SetupSPKM(false); } auto consensusParams = Params().GetConsensus(); - vpwallets.insert(vpwallets.begin(), pwalletMain.get()); + vpwallets.insert(vpwallets.begin(), &m_wallet); UniValue retValue; @@ -344,13 +344,13 @@ BOOST_AUTO_TEST_CASE(saplingOperationTests) BOOST_CHECK_NO_THROW(retValue = CallRPC("getnewaddress")); const std::string& taddrStr = retValue.get_str(); const CTxDestination& taddr1 = DecodeDestination(taddrStr); - const auto& zaddr1 = pwalletMain->GenerateNewSaplingZKey(); + const auto& zaddr1 = m_wallet.GenerateNewSaplingZKey(); std::string ret; // there are no utxos to spend { std::vector recipients = { SendManyRecipient(zaddr1, COIN, "DEADBEEF", false) }; - SaplingOperation operation(consensusParams, 1, pwalletMain.get()); + SaplingOperation operation(consensusParams, 1, &m_wallet); operation.setFromAddress(taddr1); auto res = operation.setRecipients(recipients)->buildAndSend(ret); BOOST_CHECK(!res); @@ -360,7 +360,7 @@ BOOST_AUTO_TEST_CASE(saplingOperationTests) // minconf cannot be zero when sending from zaddr { std::vector recipients = { SendManyRecipient(zaddr1, COIN, "DEADBEEF", false) }; - SaplingOperation operation(consensusParams, 1, pwalletMain.get()); + SaplingOperation operation(consensusParams, 1, &m_wallet); operation.setFromAddress(zaddr1); auto res = operation.setRecipients(recipients)->setMinDepth(0)->buildAndSend(ret); BOOST_CHECK(!res); @@ -370,7 +370,7 @@ BOOST_AUTO_TEST_CASE(saplingOperationTests) // there are no unspent notes to spend { std::vector recipients = { SendManyRecipient(taddr1, COIN, false) }; - SaplingOperation operation(consensusParams, 1, pwalletMain.get()); + SaplingOperation operation(consensusParams, 1, &m_wallet); operation.setFromAddress(zaddr1); auto res = operation.setRecipients(recipients)->buildAndSend(ret); BOOST_CHECK(!res); @@ -413,18 +413,18 @@ BOOST_AUTO_TEST_CASE(saplingOperationTests) BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling) { { - LOCK2(cs_main, pwalletMain->cs_wallet); - pwalletMain->SetupSPKM(false); + LOCK2(cs_main, m_wallet.cs_wallet); + m_wallet.SetupSPKM(false); } - vpwallets.insert(vpwallets.begin(), pwalletMain.get()); + vpwallets.insert(vpwallets.begin(), &m_wallet); UniValue retValue; // add keys manually CTxDestination taddr; - pwalletMain->getNewAddress(taddr, ""); + m_wallet.getNewAddress(taddr, ""); std::string taddr1 = EncodeDestination(taddr); - auto zaddr1 = pwalletMain->GenerateNewSaplingZKey(); + auto zaddr1 = m_wallet.GenerateNewSaplingZKey(); auto consensusParams = Params().GetConsensus(); retValue = CallRPC("getblockcount"); @@ -434,9 +434,9 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling) CMutableTransaction mtx; mtx.vout.emplace_back(5 * COIN, GetScriptForDestination(taddr)); // Add to wallet and get the updated wtx - CWalletTx wtxIn(pwalletMain.get(), MakeTransactionRef(mtx)); - pwalletMain->LoadToWallet(wtxIn); - CWalletTx& wtx = pwalletMain->mapWallet.at(mtx.GetHash()); + CWalletTx wtxIn(&m_wallet, MakeTransactionRef(mtx)); + m_wallet.LoadToWallet(wtxIn); + CWalletTx& wtx = m_wallet.mapWallet.at(mtx.GetHash()); // Fake-mine the transaction BOOST_CHECK_EQUAL(0, chainActive.Height()); @@ -452,11 +452,11 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling) chainActive.SetTip(&fakeIndex); BOOST_CHECK(chainActive.Contains(&fakeIndex)); BOOST_CHECK_EQUAL(1, chainActive.Height()); - pwalletMain->BlockConnected(std::make_shared(block), mi->second); - BOOST_CHECK_MESSAGE(pwalletMain->GetAvailableBalance() > 0, "tx not confirmed"); + m_wallet.BlockConnected(std::make_shared(block), mi->second); + BOOST_CHECK_MESSAGE(m_wallet.GetAvailableBalance() > 0, "tx not confirmed"); std::vector recipients = { SendManyRecipient(zaddr1, 1 * COIN, "ABCD", false) }; - SaplingOperation operation(consensusParams, nextBlockHeight, pwalletMain.get()); + SaplingOperation operation(consensusParams, nextBlockHeight, &m_wallet); operation.setFromAddress(taddr); BOOST_CHECK(operation.setRecipients(recipients) ->setMinDepth(0) @@ -464,7 +464,7 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling) // try from auto-selected transparent address std::vector recipients2 = { SendManyRecipient(zaddr1, 1 * COIN, "ABCD", false) }; - SaplingOperation operation2(consensusParams, nextBlockHeight, pwalletMain.get()); + SaplingOperation operation2(consensusParams, nextBlockHeight, &m_wallet); BOOST_CHECK(operation2.setSelectTransparentCoins(true) ->setRecipients(recipients2) ->setMinDepth(0) @@ -487,7 +487,7 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling) BOOST_CHECK(libzcash::AttemptSaplingOutDecryption( tx.sapData->vShieldedOutput[0].outCiphertext, - pwalletMain->GetSaplingScriptPubKeyMan()->getCommonOVK(), + m_wallet.GetSaplingScriptPubKeyMan()->getCommonOVK(), tx.sapData->vShieldedOutput[0].cv, tx.sapData->vShieldedOutput[0].cmu, tx.sapData->vShieldedOutput[0].ephemeralKey)); @@ -504,15 +504,15 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_encrypted_wallet_sapzkeys) int n = 100; { - LOCK(pwalletMain->cs_wallet); - pwalletMain->SetMinVersion(FEATURE_SAPLING); - pwalletMain->SetupSPKM(false); + LOCK(m_wallet.cs_wallet); + m_wallet.SetMinVersion(FEATURE_SAPLING); + m_wallet.SetupSPKM(false); } - vpwallets.insert(vpwallets.begin(), pwalletMain.get()); + vpwallets.insert(vpwallets.begin(), &m_wallet); // wallet should currently be empty std::set addrs; - pwalletMain->GetSaplingPaymentAddresses(addrs); + m_wallet.GetSaplingPaymentAddresses(addrs); BOOST_CHECK(addrs.empty()); // create keys @@ -535,7 +535,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_encrypted_wallet_sapzkeys) strWalletPass = "hello"; PushCurrentDirectory push_dir(gArgs.GetArg("-datadir","/tmp/thisshouldnothappen")); - BOOST_CHECK(pwalletMain->EncryptWallet(strWalletPass)); + BOOST_CHECK(m_wallet.EncryptWallet(strWalletPass)); // Verify we can still list the keys imported BOOST_CHECK_NO_THROW(retValue = CallRPC("listshieldaddresses")); @@ -547,7 +547,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_encrypted_wallet_sapzkeys) // We can't call RPC walletpassphrase as that invokes RPCRunLater which breaks tests. // So we manually unlock. - BOOST_CHECK(pwalletMain->Unlock(strWalletPass)); + BOOST_CHECK(m_wallet.Unlock(strWalletPass)); // Now add a key BOOST_CHECK_NO_THROW(CallRPC("getnewshieldaddress")); @@ -563,10 +563,10 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_encrypted_wallet_sapzkeys) BOOST_AUTO_TEST_CASE(rpc_listshieldunspent_parameters) { { - LOCK(pwalletMain->cs_wallet); - pwalletMain->SetupSPKM(false); + LOCK(m_wallet.cs_wallet); + m_wallet.SetupSPKM(false); } - vpwallets.insert(vpwallets.begin(), pwalletMain.get()); + vpwallets.insert(vpwallets.begin(), &m_wallet); UniValue retValue; diff --git a/src/test/librust/sapling_wallet_tests.cpp b/src/test/librust/sapling_wallet_tests.cpp index 915adda079a6..a7b76e04b8e3 100644 --- a/src/test/librust/sapling_wallet_tests.cpp +++ b/src/test/librust/sapling_wallet_tests.cpp @@ -74,7 +74,7 @@ BOOST_FIXTURE_TEST_SUITE(sapling_wallet_tests, WalletRegTestingSetup) BOOST_AUTO_TEST_CASE(SetSaplingNoteAddrsInCWalletTx) { auto consensusParams = Params().GetConsensus(); - CWallet& wallet = *pwalletMain; + CWallet& wallet = m_wallet; LOCK(wallet.cs_wallet); setupWallet(wallet); @@ -142,7 +142,7 @@ BOOST_AUTO_TEST_CASE(FindMySaplingNotes) { auto consensusParams = Params().GetConsensus(); - CWallet& wallet = *pwalletMain; + CWallet& wallet = m_wallet; LOCK(wallet.cs_wallet); wallet.SetupSPKM(false); @@ -179,7 +179,7 @@ BOOST_AUTO_TEST_CASE(GetConflictedSaplingNotes) { auto consensusParams = Params().GetConsensus(); - CWallet& wallet = *pwalletMain; + CWallet& wallet = m_wallet; LOCK2(cs_main, wallet.cs_wallet); setupWallet(wallet); @@ -299,7 +299,7 @@ BOOST_AUTO_TEST_CASE(SaplingNullifierIsSpent) { auto consensusParams = Params().GetConsensus(); - CWallet& wallet = *pwalletMain; + CWallet& wallet = m_wallet; LOCK2(cs_main, wallet.cs_wallet); setupWallet(wallet); @@ -358,7 +358,7 @@ BOOST_AUTO_TEST_CASE(NavigateFromSaplingNullifierToNote) { auto consensusParams = Params().GetConsensus(); - CWallet& wallet = *pwalletMain; + CWallet& wallet = m_wallet; LOCK2(cs_main, wallet.cs_wallet); setupWallet(wallet); @@ -454,7 +454,7 @@ BOOST_AUTO_TEST_CASE(SpentSaplingNoteIsFromMe) { auto consensusParams = Params().GetConsensus(); - CWallet& wallet = *pwalletMain; + CWallet& wallet = m_wallet; LOCK2(cs_main, wallet.cs_wallet); setupWallet(wallet); @@ -599,7 +599,7 @@ BOOST_AUTO_TEST_CASE(CachedWitnessesEmptyChain) { auto consensusParams = Params().GetConsensus(); - CWallet& wallet = *pwalletMain; + CWallet& wallet = m_wallet; { LOCK(wallet.cs_wallet); setupWallet(wallet); @@ -640,7 +640,7 @@ BOOST_AUTO_TEST_CASE(CachedWitnessesChainTip) auto consensusParams = Params().GetConsensus(); libzcash::SaplingExtendedSpendingKey sk = GetTestMasterSaplingSpendingKey(); - CWallet& wallet = *pwalletMain; + CWallet& wallet = m_wallet; { LOCK(wallet.cs_wallet); setupWallet(wallet); @@ -719,7 +719,7 @@ BOOST_AUTO_TEST_CASE(CachedWitnessesDecrementFirst) { auto consensusParams = Params().GetConsensus(); libzcash::SaplingExtendedSpendingKey sk = GetTestMasterSaplingSpendingKey(); - CWallet& wallet = *pwalletMain; + CWallet& wallet = m_wallet; { LOCK(wallet.cs_wallet); setupWallet(wallet); @@ -788,7 +788,7 @@ BOOST_AUTO_TEST_CASE(CachedWitnessesCleanIndex) auto consensusParams = Params().GetConsensus(); libzcash::SaplingExtendedSpendingKey sk = GetTestMasterSaplingSpendingKey(); - CWallet& wallet = *pwalletMain; + CWallet& wallet = m_wallet; { LOCK(wallet.cs_wallet); setupWallet(wallet); @@ -865,7 +865,7 @@ BOOST_AUTO_TEST_CASE(ClearNoteWitnessCache) auto consensusParams = Params().GetConsensus(); libzcash::SaplingExtendedSpendingKey sk = GetTestMasterSaplingSpendingKey(); - CWallet& wallet = *pwalletMain; + CWallet& wallet = m_wallet; { LOCK(wallet.cs_wallet); setupWallet(wallet); @@ -913,7 +913,7 @@ BOOST_AUTO_TEST_CASE(UpdatedSaplingNoteData) { auto consensusParams = Params().GetConsensus(); - CWallet& wallet = *pwalletMain; + CWallet& wallet = m_wallet; // Need to lock cs_main for now due the lock ordering. future: revamp all of this function to only lock where is needed. LOCK2(cs_main, wallet.cs_wallet); setupWallet(wallet); @@ -1024,7 +1024,7 @@ BOOST_AUTO_TEST_CASE(MarkAffectedSaplingTransactionsDirty) { auto consensusParams = Params().GetConsensus(); - CWallet& wallet = *pwalletMain; + CWallet& wallet = m_wallet; LOCK2(cs_main, wallet.cs_wallet); setupWallet(wallet); @@ -1134,7 +1134,7 @@ BOOST_AUTO_TEST_CASE(GetNotes) { auto consensusParams = Params().GetConsensus(); - CWallet& wallet = *pwalletMain; + CWallet& wallet = m_wallet; libzcash::SaplingPaymentAddress pk; uint256 blockHash; std::vector saplingOutpoints; diff --git a/src/test/librust/wallet_zkeys_tests.cpp b/src/test/librust/wallet_zkeys_tests.cpp index 19764a4cfd56..af5ff99fb78c 100644 --- a/src/test/librust/wallet_zkeys_tests.cpp +++ b/src/test/librust/wallet_zkeys_tests.cpp @@ -9,6 +9,7 @@ #include "sapling/address.h" #include "wallet/wallet.h" #include "wallet/walletdb.h" +#include "wallet/walletutil.h" #include "util/system.h" #include @@ -32,7 +33,7 @@ BOOST_FIXTURE_TEST_SUITE(wallet_zkeys_tests, WalletTestingSetup) BOOST_AUTO_TEST_CASE(StoreAndLoadSaplingZkeys) { SelectParams(CBaseChainParams::MAIN); - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); LOCK(wallet.cs_wallet); // wallet should be empty std::set addrs; @@ -135,42 +136,45 @@ BOOST_AUTO_TEST_CASE(StoreAndLoadSaplingZkeys) { BOOST_AUTO_TEST_CASE(WriteCryptedSaplingZkeyDirectToDb) { SelectParams(CBaseChainParams::TESTNET); - BOOST_CHECK(!pwalletMain->HasSaplingSPKM()); - assert(pwalletMain->SetupSPKM(true)); + fs::path path = fs::absolute("testWallet1", GetWalletDir()); + CWallet testWallet("testWallet1", CWalletDBWrapper::Create(path)); + bool fFirstRun; + BOOST_CHECK_EQUAL(testWallet.LoadWallet(fFirstRun), DB_LOAD_OK); + BOOST_CHECK(!testWallet.HasSaplingSPKM()); + assert(testWallet.SetupSPKM(true)); // wallet should be empty std::set addrs; - pwalletMain->GetSaplingPaymentAddresses(addrs); + testWallet.GetSaplingPaymentAddresses(addrs); BOOST_CHECK_EQUAL(0, addrs.size()); // Add random key to the wallet - auto address = pwalletMain->GenerateNewSaplingZKey(); + auto address = testWallet.GenerateNewSaplingZKey(); // wallet should have one key - pwalletMain->GetSaplingPaymentAddresses(addrs); + testWallet.GetSaplingPaymentAddresses(addrs); BOOST_CHECK_EQUAL(1, addrs.size()); // encrypt wallet SecureString strWalletPass; strWalletPass.reserve(100); strWalletPass = "hello"; - BOOST_CHECK(pwalletMain->EncryptWallet(strWalletPass)); + BOOST_CHECK(testWallet.EncryptWallet(strWalletPass)); // adding a new key will fail as the wallet is locked - BOOST_CHECK_THROW(pwalletMain->GenerateNewSaplingZKey(), std::runtime_error); + BOOST_CHECK_THROW(testWallet.GenerateNewSaplingZKey(), std::runtime_error); // unlock wallet and then add - pwalletMain->Unlock(strWalletPass); - libzcash::SaplingPaymentAddress address2 = pwalletMain->GenerateNewSaplingZKey(); + testWallet.Unlock(strWalletPass); + libzcash::SaplingPaymentAddress address2 = testWallet.GenerateNewSaplingZKey(); // Create a new wallet from the existing wallet path - bool fFirstRun; - std::unique_ptr dbw(new CWalletDBWrapper(&bitdb, pwalletMain->GetDBHandle().GetName())); - CWallet wallet2(std::move(dbw)); + fFirstRun = false; + CWallet wallet2("testWallet1", CWalletDBWrapper::Create(path)); BOOST_CHECK_EQUAL(DB_LOAD_OK, wallet2.LoadWallet(fFirstRun)); // Confirm it's not the same as the other wallet - BOOST_CHECK(pwalletMain.get() != &wallet2); + BOOST_CHECK(&testWallet != &wallet2); BOOST_CHECK(wallet2.HasSaplingSPKM()); // wallet should have two keys diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index bf447f336402..8e25bee911c4 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -195,7 +195,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) Checkpoints::fEnabled = false; // Simple block creation, nothing special yet: - BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, pwalletMain.get(), false)); + BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, &m_wallet, false)); // Set genesis block pblocktemplate->block.hashPrevBlock = chainparams.GetConsensus().hashGenesisBlock; @@ -221,7 +221,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) } // Just to make sure we can still make simple blocks - BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, pwalletMain.get(), false)); + BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, &m_wallet, false)); // block sigops > limit: 2000 CHECKMULTISIG + 1 tx.vin.resize(1); @@ -239,7 +239,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) mempool.addUnchecked(hash, entry.Fee(1000000).Time(GetTime()).SpendsCoinbaseOrCoinstake(spendsCoinbase).FromTx(tx)); tx.vin[0].prevout.hash = hash; } - BOOST_CHECK_EXCEPTION(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, pwalletMain.get(), false), std::runtime_error, HasReason("bad-blk-sigops")); + BOOST_CHECK_EXCEPTION(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, &m_wallet, false), std::runtime_error, HasReason("bad-blk-sigops")); mempool.clear(); tx.vin[0].prevout.hash = txFirst[0]->GetHash(); @@ -252,7 +252,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) mempool.addUnchecked(hash, entry.Fee(1000000).Time(GetTime()).SpendsCoinbaseOrCoinstake(spendsCoinbase).SigOps(20).FromTx(tx)); tx.vin[0].prevout.hash = hash; } - BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, pwalletMain.get(), false)); + BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, &m_wallet, false)); mempool.clear(); // block size > limit @@ -272,13 +272,13 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) mempool.addUnchecked(hash, entry.Fee(1000000).Time(GetTime()).SpendsCoinbaseOrCoinstake(spendsCoinbase).FromTx(tx)); tx.vin[0].prevout.hash = hash; } - BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, pwalletMain.get(), false)); + BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, &m_wallet, false)); mempool.clear(); // orphan in mempool, template creation fails hash = tx.GetHash(); mempool.addUnchecked(hash, entry.Fee(1000000).Time(GetTime()).FromTx(tx)); - BOOST_CHECK_EXCEPTION(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, pwalletMain.get(), false), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); + BOOST_CHECK_EXCEPTION(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, &m_wallet, false), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); mempool.clear(); // child with higher feerate than parent @@ -295,7 +295,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vout[0].nValue = 5900000000LL; hash = tx.GetHash(); mempool.addUnchecked(hash, entry.Fee(400000000LL).Time(GetTime()).SpendsCoinbaseOrCoinstake(true).FromTx(tx)); - BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, pwalletMain.get(), false)); + BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, &m_wallet, false)); mempool.clear(); // coinbase in mempool, template creation fails @@ -306,7 +306,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) hash = tx.GetHash(); // give it a fee so it'll get mined mempool.addUnchecked(hash, entry.Fee(100000).Time(GetTime()).SpendsCoinbaseOrCoinstake(false).FromTx(tx)); - BOOST_CHECK_EXCEPTION(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, pwalletMain.get(), false), std::runtime_error, HasReason("bad-cb-multiple")); + BOOST_CHECK_EXCEPTION(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, &m_wallet, false), std::runtime_error, HasReason("bad-cb-multiple")); mempool.clear(); // invalid (pre-p2sh) txn in mempool, template creation fails @@ -324,7 +324,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) hash = tx.GetHash(); mempool.addUnchecked(hash, entry.Fee(1000000).Time(GetTime()).SpendsCoinbaseOrCoinstake(false).FromTx(tx)); // Should throw block-validation-failed - BOOST_CHECK_EXCEPTION(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, pwalletMain.get(), false), std::runtime_error, HasReason("block-validation-failed")); + BOOST_CHECK_EXCEPTION(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, &m_wallet, false), std::runtime_error, HasReason("block-validation-failed")); mempool.clear(); // double spend txn pair in mempool, template creation fails @@ -337,7 +337,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) tx.vout[0].scriptPubKey = CScript() << OP_2; hash = tx.GetHash(); mempool.addUnchecked(hash, entry.Fee(100000000L).Time(GetTime()).SpendsCoinbaseOrCoinstake(true).FromTx(tx)); - BOOST_CHECK_EXCEPTION(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, pwalletMain.get(), false), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); + BOOST_CHECK_EXCEPTION(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, &m_wallet, false), std::runtime_error, HasReason("bad-txns-inputs-missingorspent")); mempool.clear(); // non-final txs in mempool @@ -368,7 +368,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) mempool.addUnchecked(hash, entry.Fee(100000000L).Time(GetTime()).SpendsCoinbaseOrCoinstake(true).FromTx(tx2)); { LOCK(cs_main); BOOST_CHECK(!CheckFinalTx(MakeTransactionRef(tx2), LOCKTIME_MEDIAN_TIME_PAST)); } - BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, pwalletMain.get(), false)); + BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, &m_wallet, false)); // Neither tx should have make it into the template. BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 1); @@ -385,7 +385,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) //BOOST_CHECK(CheckFinalTx(tx)); //BOOST_CHECK(CheckFinalTx(tx2)); - BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, pwalletMain.get(), false)); + BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), DEFAULT_PRINTPRIORITY).CreateNewBlock(scriptPubKey, &m_wallet, false)); BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 3); WITH_LOCK(cs_main, chainActive.Tip()->nHeight--); diff --git a/src/test/script_P2CS_tests.cpp b/src/test/script_P2CS_tests.cpp index 47b6d2576f46..e7f3999cb2f8 100644 --- a/src/test/script_P2CS_tests.cpp +++ b/src/test/script_P2CS_tests.cpp @@ -303,7 +303,7 @@ static void setupWallet(CWallet& wallet) BOOST_AUTO_TEST_CASE(fake_script_test) { - CWallet& wallet = *pwalletMain; + CWallet& wallet = m_wallet; LOCK(wallet.cs_wallet); setupWallet(wallet); CKey stakerKey; // dummy staker key (not in the wallet) diff --git a/src/test/validation_tests.cpp b/src/test/validation_tests.cpp index 8eef74aa1fa4..7491f285a4fd 100644 --- a/src/test/validation_tests.cpp +++ b/src/test/validation_tests.cpp @@ -123,7 +123,7 @@ BOOST_FIXTURE_TEST_CASE(zerocoin_rejection_tests, WalletRegTestingSetup) std::unique_ptr pblocktemplate; CScript scriptPubKey = CScript() << OP_DUP << OP_HASH160 << ParseHex("8d5b4f83212214d6ef693e02e6d71969fddad976") << OP_EQUALVERIFY << OP_CHECKSIG; - BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), false).CreateNewBlock(scriptPubKey, pwalletMain.get(), false)); + BOOST_CHECK(pblocktemplate = BlockAssembler(Params(), false).CreateNewBlock(scriptPubKey, &m_wallet, false)); pblocktemplate->block.hashPrevBlock = chainparams.GetConsensus().hashGenesisBlock; // Base tx diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index b00671b6652b..33aadc689c7f 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -55,20 +55,44 @@ void CheckUniqueFileid(const CDBEnv& env, const std::string& filename, Db& db) } } } + +RecursiveMutex cs_db; +std::map g_dbenvs; //!< Map from directory name to open db environment. } // namespace +CDBEnv* GetWalletEnv(const fs::path& wallet_path, std::string& database_filename) +{ + fs::path env_directory = wallet_path.parent_path(); + database_filename = wallet_path.filename().string(); + LOCK(cs_db); + // Note: An ununsed temporary CDBEnv object may be created inside the + // emplace function if the key already exists. This is a little inefficient, + // but not a big concern since the map will be changed in the future to hold + // pointers instead of objects, anyway. + return &g_dbenvs.emplace(std::piecewise_construct, std::forward_as_tuple(env_directory.string()), std::forward_as_tuple(env_directory)).first->second; +} + // // CDB // -CDBEnv bitdb; - -void CDBEnv::EnvShutdown() +void CDBEnv::Close() { if (!fDbEnvInit) return; fDbEnvInit = false; + + for (auto& db : mapDb) { + auto count = mapFileUseCount.find(db.first); + assert(count == mapFileUseCount.end() || count->second == 0); + if (db.second) { + db.second->close(0); + delete db.second; + db.second = nullptr; + } + } + int ret = dbenv->close(0); if (ret != 0) LogPrintf("CDBEnv::EnvShutdown : Error %d shutting down database environment: %s\n", ret, DbEnv::strerror(ret)); @@ -83,29 +107,24 @@ void CDBEnv::Reset() fMockDb = false; } -CDBEnv::CDBEnv() +CDBEnv::CDBEnv(const fs::path& dir_path) : strPath(dir_path.string()) { Reset(); } CDBEnv::~CDBEnv() { - EnvShutdown(); -} - -void CDBEnv::Close() -{ - EnvShutdown(); + Close(); } -bool CDBEnv::Open(const fs::path& pathIn, bool retry) +bool CDBEnv::Open(bool retry) { if (fDbEnvInit) return true; boost::this_thread::interruption_point(); - strPath = pathIn.string(); + fs::path pathIn = strPath; if (!LockDirectory(pathIn, ".walletlock")) { LogPrintf("Cannot obtain a lock on wallet directory %s. Another instance of PIVX may be using it.\n", strPath); return false; @@ -153,7 +172,7 @@ bool CDBEnv::Open(const fs::path& pathIn, bool retry) // failure is ok (well, not really, but it's not worse than what we started with) } // try opening it again one more time - if (!Open(pathIn, false)) { + if (!Open(false /* retry */)) { // if it still fails, it probably means we can't even create the database env return false; } @@ -212,12 +231,15 @@ CDBEnv::VerifyResult CDBEnv::Verify(const std::string& strFile, recoverFunc_type return RECOVER_FAIL; // Try to recover: - bool fRecovered = (*recoverFunc)(strFile, out_backup_filename); + bool fRecovered = (*recoverFunc)(fs::path(strPath) / strFile, out_backup_filename); return (fRecovered ? RECOVER_OK : RECOVER_FAIL); } -bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& newFilename) +bool CDB::Recover(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& newFilename) { + std::string filename; + CDBEnv* env = GetWalletEnv(file_path, filename); + // Recovery procedure: // move wallet file to walletfilename.timestamp.bak // Call Salvage with fAggressive=true to @@ -228,7 +250,7 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco int64_t now = GetTime(); newFilename = strprintf("%s.%d.bak", filename, now); - int result = bitdb.dbenv->dbrename(NULL, filename.c_str(), NULL, + int result = env->dbenv->dbrename(nullptr, filename.c_str(), nullptr, newFilename.c_str(), DB_AUTO_COMMIT); if (result == 0) { LogPrintf("Renamed %s to %s\n", filename, newFilename); @@ -238,14 +260,15 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco } std::vector salvagedData; - bool fSuccess = bitdb.Salvage(newFilename, true, salvagedData); - if (salvagedData.empty()) { + bool fSuccess = env->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 pdbCopy(new Db(bitdb.dbenv.get(), 0)); + std::unique_ptr pdbCopy = std::make_unique(env->dbenv.get(), 0); int ret = pdbCopy->open(nullptr, // Txn pointer filename.c_str(), // Filename "main", // Logical db name @@ -258,9 +281,11 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco return false; } - DbTxn* ptxn = bitdb.TxnBegin(); - for (CDBEnv::KeyValPair& row : salvagedData) { - if (recoverKVcallback) { + DbTxn* ptxn = env->TxnBegin(); + for (CDBEnv::KeyValPair& row : salvagedData) + { + if (recoverKVcallback) + { CDataStream ssKey(row.first, SER_DISK, CLIENT_VERSION); CDataStream ssValue(row.second, SER_DISK, CLIENT_VERSION); if (!(*recoverKVcallback)(callbackDataIn, ssKey, ssValue)) @@ -278,8 +303,12 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco return fSuccess; } -bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr) +bool CDB::VerifyEnvironment(const fs::path& file_path, std::string& errorStr) { + std::string walletFile; + CDBEnv* env = GetWalletEnv(file_path, walletFile); + fs::path walletDir = env->Directory(); + LogPrintf("Using BerkeleyDB version %s\n", DbEnv::version(0, 0, 0)); LogPrintf("Using wallet %s\n", walletFile); @@ -288,7 +317,7 @@ bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walle if (walletFile != wallet_file_path.filename().string()) return UIError(strprintf(_("Wallet %s resides outside data directory %s"), walletFile, walletDir.string())); - if (!bitdb.Open(walletDir, true)) { + if (!env->Open(true /* retry */)) { errorStr = strprintf(_("Error initializing wallet database environment %s!"), walletDir); return false; } @@ -296,13 +325,18 @@ bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walle return true; } -bool CDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc) +bool CDB::VerifyDatabaseFile(const fs::path& file_path, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc) { + std::string walletFile; + CDBEnv* env = GetWalletEnv(file_path, walletFile); + fs::path walletDir = env->Directory(); + if (fs::exists(walletDir / walletFile)) { std::string backup_filename; - CDBEnv::VerifyResult r = bitdb.Verify(walletFile, recoverFunc, backup_filename); - if (r == CDBEnv::RECOVER_OK) { + CDBEnv::VerifyResult r = env->Verify(walletFile, recoverFunc, backup_filename); + 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" @@ -409,8 +443,8 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb nFlags |= DB_CREATE; { - LOCK(env->cs_db); - if (!env->Open(GetWalletDir())) + LOCK(cs_db); + if (!env->Open(false /* retry */)) throw std::runtime_error("CDB: Failed to open database environment."); pdb = env->mapDb[strFilename]; @@ -437,7 +471,25 @@ CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb if (ret != 0) { throw std::runtime_error(strprintf("CDB: Error %d, can't open database %s", ret, strFilename)); } - CheckUniqueFileid(*env, strFilename, *pdb_temp); + + // Call CheckUniqueFileid on the containing BDB environment to + // avoid BDB data consistency bugs that happen when different data + // files in the same environment have the same fileid. + // + // Also call CheckUniqueFileid on all the other g_dbenvs to prevent + // PIVX from opening the same data file through another + // environment when the file is referenced through equivalent but + // not obviously identical symlinked or hard linked or bind mounted + // paths. In the future a more relaxed check for equal inode and + // device ids could be done instead, which would allow opening + // different backup copies of a wallet at the same time. Maybe even + // more ideally, an exclusive lock for accessing the database could + // be implemented, so no equality checks are needed at all. (Newer + // versions of BDB have an set_lk_exclusive method for this + // purpose, but the older version we use does not.) + for (auto& env : g_dbenvs) { + CheckUniqueFileid(env.second, strFilename, *pdb_temp); + } pdb = pdb_temp.release(); env->mapDb[strFilename] = pdb; @@ -485,7 +537,7 @@ void CDB::Close() Flush(); { - LOCK(env->cs_db); + LOCK(cs_db); --env->mapFileUseCount[strFile]; } } @@ -513,7 +565,7 @@ bool CDB::Rewrite(CWalletDBWrapper& dbw, const char* pszSkip) const std::string& strFile = dbw.strFile; while (true) { { - LOCK(env->cs_db); + LOCK(cs_db); if (!env->mapFileUseCount.count(strFile) || env->mapFileUseCount[strFile] == 0) { // Flush log data to the dat file env->CloseDb(strFile); @@ -643,7 +695,7 @@ bool CDB::PeriodicFlush(CWalletDBWrapper& dbw) bool ret = false; CDBEnv *env = dbw.env; const std::string& strFile = dbw.strFile; - TRY_LOCK(bitdb.cs_db,lockDb); + TRY_LOCK(cs_db, lockDb); if (lockDb) { // Don't do this if any databases are in use @@ -688,7 +740,7 @@ bool CWalletDBWrapper::Backup(const std::string& strDest) while (true) { { - LOCK(env->cs_db); + LOCK(cs_db); if (!env->mapFileUseCount.count(strFile) || env->mapFileUseCount[strFile] == 0) { // Flush log data to the dat file diff --git a/src/wallet/db.h b/src/wallet/db.h index 5254f1c41d6e..db1b0dc40b2c 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -9,6 +9,7 @@ #include "clientversion.h" #include "fs.h" +#include "util/memory.h" #include "serialize.h" #include "streams.h" #include "sync.h" @@ -33,20 +34,19 @@ class CDBEnv // shutdown problems/crashes caused by a static initialized internal pointer. std::string strPath; - void EnvShutdown(); - public: - mutable RecursiveMutex cs_db; std::unique_ptr dbenv; std::map mapFileUseCount; std::map mapDb; - CDBEnv(); + CDBEnv(const fs::path& env_directory); ~CDBEnv(); void Reset(); void MakeMock(); bool IsMock() const { return fMockDb; } + bool IsInitialized() const { return fDbEnvInit; } + fs::path Directory() const { return strPath; } /** * Verify that database file strFile is OK. If it is not, @@ -57,7 +57,7 @@ class CDBEnv enum VerifyResult { VERIFY_OK, RECOVER_OK, RECOVER_FAIL }; - typedef bool (*recoverFunc_type)(const std::string& strFile, std::string& out_backup_filename); + typedef bool (*recoverFunc_type)(const fs::path& file_path, std::string& out_backup_filename); VerifyResult Verify(const std::string& strFile, recoverFunc_type recoverFunc, std::string& out_backup_filename); /** @@ -70,7 +70,7 @@ class CDBEnv typedef std::pair, std::vector > KeyValPair; bool Salvage(const std::string& strFile, bool fAggressive, std::vector& vResult); - bool Open(const fs::path& path, bool retry = 0); + bool Open(bool retry); void Close(); void Flush(bool fShutdown); void CheckpointLSN(const std::string& strFile); @@ -87,7 +87,8 @@ class CDBEnv } }; -extern CDBEnv bitdb; +/** Get CDBEnv and database filename given a wallet path. */ +CDBEnv* GetWalletEnv(const fs::path& wallet_path, std::string& database_filename); /** An instance of this class represents one database. * For BerkeleyDB this is just a (env, strFile) tuple. @@ -102,9 +103,33 @@ class CWalletDBWrapper } /** Create DB handle to real database */ - CWalletDBWrapper(CDBEnv *env_in, const std::string &strFile_in): - nLastSeen(0), nLastFlushed(0), nLastWalletUpdate(0), env(env_in), strFile(strFile_in) + CWalletDBWrapper(const fs::path& wallet_path, bool mock = false) : + nUpdateCounter(0), nLastSeen(0), nLastFlushed(0), nLastWalletUpdate(0) { + env = GetWalletEnv(wallet_path, strFile); + if (mock) { + env->Close(); + env->Reset(); + env->MakeMock(); + } + } + + /** Return object for accessing database at specified path. */ + static std::unique_ptr Create(const fs::path& path) + { + return std::make_unique(path); + } + + /** Return object for accessing dummy database with no read/write capabilities. */ + static std::unique_ptr CreateDummy() + { + return std::make_unique(); + } + + /** Return object for accessing temporary in-memory database. */ + static std::unique_ptr CreateMock() + { + return std::make_unique("", true /* mock */); } /** Rewrite the entire database on disk, with the exception of key pszSkip if non-zero @@ -115,10 +140,6 @@ class CWalletDBWrapper */ bool Backup(const std::string& strDest); - /** Get a name for this database, for debugging etc. - */ - std::string GetName() const { return strFile; } - /** Make sure all changes are flushed to disk. */ void Flush(bool shutdown); @@ -163,15 +184,15 @@ class CDB void Flush(); void Close(); - static bool Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); + static bool Recover(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); /* flush the wallet passively (TRY_LOCK) ideal to be called periodically */ static bool PeriodicFlush(CWalletDBWrapper& dbw); /* verifies the database environment */ - static bool VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr); + static bool VerifyEnvironment(const fs::path& file_path, std::string& errorStr); /* verifies the database file */ - static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc); + static bool VerifyDatabaseFile(const fs::path& file_path, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc); public: template @@ -331,7 +352,7 @@ class CDB { if (!pdb || activeTxn) return false; - DbTxn* ptxn = bitdb.TxnBegin(); + DbTxn* ptxn = env->TxnBegin(); if (!ptxn) return false; activeTxn = ptxn; diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index d9c509e8faaf..91718c55e60c 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -183,26 +183,26 @@ bool WalletVerify() } std::string strError; - if (!CWalletDB::VerifyEnvironment(walletFile, GetWalletDir().string(), strError)) { + if (!CWalletDB::VerifyEnvironment(wallet_path, strError)) { return UIError(strError); } if (gArgs.GetBoolArg("-salvagewallet", false)) { // Recover readable keypairs: - CWallet dummyWallet; + CWallet dummyWallet("dummy", CWalletDBWrapper::CreateDummy()); std::string backup_filename; // Even if we don't use this lock in this function, we want to preserve // lock order in LoadToWallet if query of chain state is needed to know // tx status. If lock can't be taken, tx confirmation status may be not // reliable. LOCK(cs_main); - if (!CWalletDB::Recover(walletFile, (void *)&dummyWallet, CWalletDB::RecoverKeysOnlyFilter, backup_filename)) { + if (!CWalletDB::Recover(wallet_path, (void *)&dummyWallet, CWalletDB::RecoverKeysOnlyFilter, backup_filename)) { return false; } } std::string strWarning; - bool dbV = CWalletDB::VerifyDatabaseFile(walletFile, GetWalletDir().string(), strWarning, strError); + bool dbV = CWalletDB::VerifyDatabaseFile(wallet_path, strWarning, strError); if (!strWarning.empty()) { UIWarning(strWarning); } @@ -223,7 +223,7 @@ bool InitLoadWallet() for (const std::string& walletFile : gArgs.GetArgs("-wallet")) { // create/load wallet - CWallet * const pwallet = CWallet::CreateWalletFromFile(walletFile); + CWallet * const pwallet = CWallet::CreateWalletFromFile(walletFile, fs::absolute(walletFile, GetWalletDir())); if (!pwallet) { return false; } diff --git a/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp b/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp index 76c99f72ecf4..f7fd88daeea4 100644 --- a/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp +++ b/src/wallet/test/wallet_sapling_transactions_validations_tests.cpp @@ -4,8 +4,6 @@ #include "wallet/test/wallet_test_fixture.h" -#include "blockassembler.h" -#include "consensus/merkle.h" #include "primitives/block.h" #include "sapling/transaction_builder.h" #include "sapling/sapling_operation.h" @@ -25,10 +23,8 @@ struct TestSaplingChainSetup: public TestChain100Setup { initZKSNARKS(); // init zk-snarks lib - bitdb.MakeMock(); bool fFirstRun; - std::unique_ptr dbw(new CWalletDBWrapper(&bitdb, "wallet_test.dat")); - pwalletMain = MakeUnique(std::move(dbw)); + pwalletMain = MakeUnique("testWallet", CWalletDBWrapper::CreateMock()); pwalletMain->LoadWallet(fFirstRun); RegisterValidationInterface(pwalletMain.get()); @@ -53,8 +49,6 @@ struct TestSaplingChainSetup: public TestChain100Setup ~TestSaplingChainSetup() { UnregisterValidationInterface(pwalletMain.get()); - bitdb.Flush(true); - bitdb.Reset(); } }; diff --git a/src/wallet/test/wallet_shielded_balances_tests.cpp b/src/wallet/test/wallet_shielded_balances_tests.cpp index 237663b3ced9..bd9770cba065 100644 --- a/src/wallet/test/wallet_shielded_balances_tests.cpp +++ b/src/wallet/test/wallet_shielded_balances_tests.cpp @@ -49,7 +49,7 @@ CWalletTx& AddShieldedBalanceToWallet(CAmount inputAmount, { // Dummy wallet, used to generate the dummy transparent input key and sign it in the transaction builder - CWallet dummyWallet; + CWallet dummyWallet("dummy", CWalletDBWrapper::CreateDummy()); dummyWallet.SetMinVersion(FEATURE_SAPLING); dummyWallet.SetupSPKM(false, true); LOCK(dummyWallet.cs_wallet); @@ -137,7 +137,7 @@ BOOST_AUTO_TEST_CASE(GetShieldedSimpleCachedCreditAndDebit) auto consensusParams = Params().GetConsensus(); // Main wallet - CWallet &wallet = *pwalletMain; + CWallet &wallet = m_wallet; LOCK2(cs_main, wallet.cs_wallet); setupWallet(wallet); @@ -237,7 +237,7 @@ BOOST_AUTO_TEST_CASE(VerifyShieldedToRemoteShieldedCachedBalance) auto consensusParams = Params().GetConsensus(); // Main wallet - CWallet &wallet = *pwalletMain; + CWallet &wallet = m_wallet; LOCK2(cs_main, wallet.cs_wallet); setupWallet(wallet); @@ -317,7 +317,7 @@ BOOST_AUTO_TEST_CASE(GetShieldedAvailableCredit) auto consensusParams = Params().GetConsensus(); // Main wallet - CWallet &wallet = *pwalletMain; + CWallet &wallet = m_wallet; LOCK2(cs_main, wallet.cs_wallet); setupWallet(wallet); @@ -358,7 +358,7 @@ BOOST_AUTO_TEST_CASE(GetShieldedAvailableCredit) std::vector ops = {saplingEntries[0].op}; uint256 anchor; std::vector> witnesses; - pwalletMain->GetSaplingScriptPubKeyMan()->GetSaplingNoteWitnesses(ops, witnesses, anchor); + wallet.GetSaplingScriptPubKeyMan()->GetSaplingNoteWitnesses(ops, witnesses, anchor); SaplingSpendValues sapSpendValues{saplingEntries[0].note, anchor, *witnesses[0]}; // Remote destination values diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 085f29bdf925..354dfc0d2d9a 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -8,25 +8,19 @@ #include "rpc/server.h" #include "wallet/db.h" #include "wallet/rpcwallet.h" +#include "wallet/wallet.h" WalletTestingSetup::WalletTestingSetup(const std::string& chainName): - SaplingTestingSetup(chainName) + SaplingTestingSetup(chainName), m_wallet("mock", CWalletDBWrapper::CreateMock()) { - bitdb.MakeMock(); - bool fFirstRun; - std::unique_ptr dbw(new CWalletDBWrapper(&bitdb, "wallet_test.dat")); - pwalletMain = MakeUnique(std::move(dbw)); - pwalletMain->LoadWallet(fFirstRun); - RegisterValidationInterface(pwalletMain.get()); + m_wallet.LoadWallet(fFirstRun); + RegisterValidationInterface(&m_wallet); RegisterWalletRPCCommands(tableRPC); } WalletTestingSetup::~WalletTestingSetup() { - UnregisterValidationInterface(pwalletMain.get()); - - bitdb.Flush(true); - bitdb.Reset(); + UnregisterValidationInterface(&m_wallet); } diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index e9006ffd88f9..6c76d5abddc6 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -17,7 +17,7 @@ struct WalletTestingSetup : public SaplingTestingSetup WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); ~WalletTestingSetup(); - std::unique_ptr pwalletMain; + CWallet m_wallet; }; struct WalletRegTestingSetup : public WalletTestingSetup diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 722c761bbd54..1eec0a5c9736 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -10,6 +10,7 @@ #include "txmempool.h" #include "validation.h" #include "wallet/wallet.h" +#include "wallet/walletutil.h" #include #include @@ -35,7 +36,7 @@ typedef std::set > CoinSet; BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup) -static const CWallet testWallet; +static const CWallet testWallet("dummy", CWalletDBWrapper::CreateDummy()); static std::vector vCoins; static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0) @@ -337,7 +338,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) // Verify ScanForWalletTransactions picks up transactions in both the old // and new block files. { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); WITH_LOCK(wallet.cs_wallet, wallet.SetLastBlockProcessed(newTip); ); AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(&wallet); @@ -354,7 +355,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) // Verify ScanForWalletTransactions only picks transactions in the new block // file. { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); AddKey(wallet, coinbaseKey); WalletRescanReserver reserver(&wallet); reserver.reserve(); @@ -367,7 +368,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) // before the missing block, and success for a key whose creation time is // after. { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); WITH_LOCK(wallet.cs_wallet, wallet.SetLastBlockProcessed(newTip); ); vpwallets.insert(vpwallets.begin(), &wallet); UniValue keys; @@ -418,7 +419,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // Import key into wallet and call dumpwallet to create backup file. { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); { LOCK(wallet.cs_wallet); wallet.mapKeyMetadata[coinbaseKey.GetPubKey().GetID()].nCreateTime = KEY_TIME; @@ -435,7 +436,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME // were scanned, and no prior blocks were scanned. { - CWallet wallet; + CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); JSONRPCRequest request; request.params.setArray(); @@ -567,7 +568,9 @@ BOOST_AUTO_TEST_CASE(cached_balances_tests) CAmount nCredit = 20 * COIN; // Setup wallet - CWallet &wallet = *pwalletMain; + CWallet wallet("testWallet1", CWalletDBWrapper::Create(fs::absolute("testWallet1", GetWalletDir()))); + bool fFirstRun; + BOOST_CHECK_EQUAL(wallet.LoadWallet(fFirstRun), DB_LOAD_OK); LOCK2(cs_main, wallet.cs_wallet); wallet.SetMinVersion(FEATURE_PRE_SPLIT_KEYPOOL); wallet.SetupSPKM(false); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 7fa8ba8bc0f9..dd6ae5da16e5 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2065,7 +2065,7 @@ std::set CWalletTx::GetConflicts() const void CWallet::Flush(bool shutdown) { - bitdb.Flush(shutdown); + dbw->Flush(shutdown); } void CWallet::ResendWalletTransactions(CConnman* connman) @@ -4102,32 +4102,29 @@ void CWallet::LockIfMyCollateral(const CTransactionRef& ptx) } } -CWallet* CWallet::CreateWalletFromFile(const std::string& walletFile) +CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path& path) { + const std::string& walletFile = name; + // needed to restore wallet transaction meta data after -zapwallettxes std::vector vWtx; if (gArgs.GetBoolArg("-zapwallettxes", false)) { uiInterface.InitMessage(_("Zapping all transactions from wallet...")); - std::unique_ptr dbw(new CWalletDBWrapper(&bitdb, walletFile)); - CWallet *tempWallet = new CWallet(std::move(dbw)); + std::unique_ptr tempWallet = std::make_unique(name, CWalletDBWrapper::Create(path)); DBErrors nZapWalletRet = tempWallet->ZapWalletTx(vWtx); if (nZapWalletRet != DB_LOAD_OK) { UIError(strprintf(_("Error loading %s: Wallet corrupted"), walletFile)); return nullptr; } - - delete tempWallet; - tempWallet = nullptr; } uiInterface.InitMessage(_("Loading wallet...")); int64_t nStart = GetTimeMillis(); bool fFirstRun = true; - std::unique_ptr dbw(new CWalletDBWrapper(&bitdb, walletFile)); - CWallet *walletInstance = new CWallet(std::move(dbw)); + CWallet *walletInstance = new CWallet(name, CWalletDBWrapper::Create(path)); DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); if (nLoadWalletRet != DB_LOAD_OK) { if (nLoadWalletRet == DB_CORRUPT) { @@ -4372,16 +4369,10 @@ bool CWalletTx::AcceptToMemoryPool(CValidationState& state) std::string CWallet::GetUniqueWalletBackupName() const { - return strprintf("%s%s", (dbw ? dbw->GetName() : "null"), FormatISO8601DateTimeForBackup(GetTime())); -} - -CWallet::CWallet() : dbw(new CWalletDBWrapper()) -{ - SetNull(); + return strprintf("%s%s", (!m_name.empty() ? m_name : "null"), FormatISO8601DateTimeForBackup(GetTime())); } -CWallet::CWallet(std::unique_ptr dbw_in) - : dbw(std::move(dbw_in)) +CWallet::CWallet(std::string name, std::unique_ptr dbw) : m_name(std::move(name)), dbw(std::move(dbw)) { SetNull(); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index ff608b813410..32a9f7b78fee 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -593,6 +593,14 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface //! the maximum wallet format version: memory-only variable that specifies to what version this wallet may be upgraded int nWalletMaxVersion; + /** + * Wallet filename from wallet= command line or config option. + * Used in debug logs and to send RPCs to the right wallet instance when + * more than one wallet is loaded. + */ + std::string m_name; + + /** Internal database handle. */ std::unique_ptr dbw; /** @@ -737,19 +745,10 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface /** Get a name for this wallet for logging/debugging purposes. */ - std::string GetName() const - { - if (dbw) { - return dbw->GetName(); - } else { - return "dummy"; - } - } + const std::string& GetName() const { return m_name; } - // Create wallet with dummy database handle - CWallet(); - // Create wallet with passed-in database handle - CWallet(std::unique_ptr dbw_in); + /** Construct wallet with specified name and database implementation. */ + CWallet(std::string name, std::unique_ptr dbw_in); ~CWallet(); void SetNull(); @@ -1168,7 +1167,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface bool AbandonTransaction(const uint256& hashTx); /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ - static CWallet* CreateWalletFromFile(const std::string& walletFile); + static CWallet* CreateWalletFromFile(const std::string& name, const fs::path& path); /** * Wallet post-init setup diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index d150319b177f..24971e5a917f 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -875,7 +875,7 @@ static bool cleanWalletBackups(folder_set_t& folder_set, int nWalletBackups, std bool AutoBackupWallet(const CWallet& wallet, std::string& strBackupWarning, std::string& strBackupError) { - std::string strWalletFile = wallet.GetDBHandle().GetName(); + std::string strWalletFile = wallet.GetName(); strBackupWarning = strBackupError = ""; int nWalletBackups = std::max(0, std::min(10, (int)gArgs.GetArg("-createwalletbackups", DEFAULT_CREATEWALLETBACKUPS))); @@ -889,7 +889,7 @@ bool AutoBackupWallet(const CWallet& wallet, std::string& strBackupWarning, std: TryCreateDirectories(backupsDir); // Copy wallet file - fs::path sourceFile = GetDataDir() / strWalletFile; + fs::path sourceFile = GetWalletDir() / strWalletFile; fs::path backupFile = backupsDir / (strWalletFile + FormatISO8601DateTimeForBackup(GetTime())); sourceFile.make_preferred(); backupFile.make_preferred(); @@ -899,7 +899,7 @@ bool AutoBackupWallet(const CWallet& wallet, std::string& strBackupWarning, std: } if (!fs::exists(sourceFile)) { - strBackupWarning = _("Failed to create backup, wallet file not found"); + strBackupWarning = strprintf(_("Failed to create backup, wallet file not found, path %s"), sourceFile); LogPrintf("%s\n", strBackupWarning); return false; } @@ -950,16 +950,16 @@ bool AttemptBackupWallet(const CWallet* wallet, const fs::path& pathSrc, const f // // Try to (very carefully!) recover wallet file if there is a problem. // -bool CWalletDB::Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename) +bool CWalletDB::Recover(const fs::path& wallet_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename) { - return CDB::Recover(filename, callbackDataIn, recoverKVcallback, out_backup_filename); + return CDB::Recover(wallet_path, callbackDataIn, recoverKVcallback, out_backup_filename); } -bool CWalletDB::Recover(const std::string& filename, std::string& out_backup_filename) +bool CWalletDB::Recover(const fs::path& wallet_path, std::string& out_backup_filename) { // recover without a key filter callback // results in recovering all record types - return CWalletDB::Recover(filename, NULL, NULL, out_backup_filename); + return CWalletDB::Recover(wallet_path, nullptr, nullptr, out_backup_filename); } bool CWalletDB::RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDataStream ssValue) @@ -985,14 +985,14 @@ bool CWalletDB::RecoverKeysOnlyFilter(void *callbackData, CDataStream ssKey, CDa return true; } -bool CWalletDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr) +bool CWalletDB::VerifyEnvironment(const fs::path& wallet_path, std::string& errorStr) { - return CDB::VerifyEnvironment(walletFile, walletDir, errorStr); + return CDB::VerifyEnvironment(wallet_path, errorStr); } -bool CWalletDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr) +bool CWalletDB::VerifyDatabaseFile(const fs::path& wallet_path, std::string& warningStr, std::string& errorStr) { - return CDB::VerifyDatabaseFile(walletFile, walletDir, warningStr, errorStr, CWalletDB::Recover); + return CDB::VerifyDatabaseFile(wallet_path, warningStr, errorStr, CWalletDB::Recover); } bool CWalletDB::WriteDestData(const std::string& address, const std::string& key, const std::string& value) diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 6da18d60b6a1..1e88b537750b 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -204,17 +204,17 @@ class CWalletDB DBErrors FindWalletTx(CWallet* pwallet, std::vector& vTxHash, std::vector& vWtx); DBErrors ZapWalletTx(CWallet* pwallet, std::vector& vWtx); /* Try to (very carefully!) recover wallet database (with a possible key type filter) */ - static bool Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); + static bool Recover(const fs::path& wallet_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename); /* Recover convenience-function to bypass the key filter callback, called when verify fails, recovers everything */ - static bool Recover(const std::string& filename, std::string& out_backup_filename); + static bool Recover(const fs::path& wallet_path, std::string& out_backup_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 fs::path& walletDir, std::string& errorStr); + static bool VerifyEnvironment(const fs::path& wallet_path, std::string& errorStr); /* verifies the database file */ - static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr); + static bool VerifyDatabaseFile(const fs::path& wallet_path, std::string& warningStr, std::string& errorStr); //! Begin a new transaction bool TxnBegin(); diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index 3d693e4d33e3..e66a1cd34447 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -51,7 +51,7 @@ def run_test(self): os.mkdir(new_data_dir_2) self.nodes[0].datadir = new_data_dir_2 self.start_node(0, ['-datadir='+new_data_dir_2, '-conf='+conf_file, '-wallet=w2']) - assert os.path.isfile(os.path.join(new_data_dir_2, 'regtest', 'w2')) + assert os.path.isfile(os.path.join(new_data_dir_2, 'regtest', 'wallets', 'w2')) if __name__ == '__main__': ConfArgsTest().main() diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 71d249a46cf2..ceb4fa94562b 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -520,7 +520,7 @@ def cache_path(n, *paths): for i in range(MAX_NODES): for entry in os.listdir(cache_path(i)): - if entry not in ['wallet.dat', 'chainstate', 'blocks', 'sporks', 'evodb', 'zerocoin', 'backups']: + if entry not in ['wallet.dat', 'chainstate', 'blocks', 'sporks', 'evodb', 'zerocoin', 'backups', "wallets"]: os.remove(cache_path(i, entry)) def clean_cache_dir(): diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index dda940f08924..6fe76133a548 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -72,8 +72,6 @@ def run_test(self): self.assert_start_raises_init_error(0, ['-upgradewallet', '-wallet=w1', '-wallet=w2'], "Error: -upgradewallet is only allowed with a single wallet file") self.assert_start_raises_init_error(0, ['-upgradewallet=1', '-wallet=w1', '-wallet=w2'], "Error: -upgradewallet is only allowed with a single wallet file") - # !TODO: backport bitcoin#12220 - # if wallets/ doesn't exist, datadir should be the default wallet dir wallet_dir2 = data_dir('walletdir') os.rename(wallet_dir(), wallet_dir2) @@ -88,7 +86,7 @@ def run_test(self): assert_equal(set(node.listwallets()), {"w4", "w5"}) w5 = wallet("w5") w5_info = w5.getwalletinfo() - assert_equal(w5_info['immature_balance'], 50) + assert_equal(w5_info['immature_balance'], 250) competing_wallet_dir = os.path.join(self.options.tmpdir, 'competing_walletdir') os.mkdir(competing_wallet_dir) diff --git a/test/functional/wallet_reorgsrestore.py b/test/functional/wallet_reorgsrestore.py index a9d290f7e697..6c00075b0b13 100755 --- a/test/functional/wallet_reorgsrestore.py +++ b/test/functional/wallet_reorgsrestore.py @@ -91,7 +91,7 @@ def run_test(self): # Node0 wallet file is loaded on longest sync'ed node1 self.stop_node(1) self.nodes[0].backupwallet(os.path.join(self.nodes[0].datadir, 'wallet.bak')) - shutil.copyfile(os.path.join(self.nodes[0].datadir, 'wallet.bak'), os.path.join(self.nodes[1].datadir, 'regtest', 'wallet.dat')) + shutil.copyfile(os.path.join(self.nodes[0].datadir, 'wallet.bak'), os.path.join(self.nodes[1].datadir, 'regtest', "wallets", 'wallet.dat')) self.start_node(1) tx_after_reorg = self.nodes[1].gettransaction(txid) # Check that normal confirmed tx is confirmed again but with different blockhash From daa7fe57ab630460d9ff7fb7db477a017251f014 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Tue, 14 Nov 2017 13:32:41 -0500 Subject: [PATCH 16/32] Allow wallet files not in -walletdir directory Remove restriction that -wallet filenames can only refer to files in the -walletdir directory. --- doc/release-notes.md | 9 +++++ src/pivx-cli.cpp | 2 +- src/wallet/db.cpp | 1 + src/wallet/init.cpp | 9 +---- test/functional/wallet_multiwallet.py | 57 ++++++++++++++------------- 5 files changed, 42 insertions(+), 36 deletions(-) diff --git a/doc/release-notes.md b/doc/release-notes.md index 6067946c925c..04dc8f743da4 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -320,6 +320,15 @@ PIVX data directory. The behavior is now: Care should be taken when choosing the wallets directory location, as if it becomes unavailable during operation, funds may be lost. +External wallet files +--------------------- + +The `-wallet=` option now accepts full paths instead of requiring wallets +to be located in the -walletdir directory. When wallets are located in +different directories, wallet data will be stored independently, so data from +every wallet is not mixed into the same /database/log.?????????? +files. + Database cache memory increased -------------------------------- diff --git a/src/pivx-cli.cpp b/src/pivx-cli.cpp index ce15f3a8629d..a3eb2c9298e5 100644 --- a/src/pivx-cli.cpp +++ b/src/pivx-cli.cpp @@ -43,7 +43,7 @@ std::string HelpMessageCli() strUsage += HelpMessageOpt("-rpcuser=", _("Username for JSON-RPC connections")); strUsage += HelpMessageOpt("-rpcpassword=", _("Password for JSON-RPC connections")); strUsage += HelpMessageOpt("-rpcclienttimeout=", strprintf(_("Timeout in seconds during HTTP requests, or 0 for no timeout. (default: %d)"), DEFAULT_HTTP_CLIENT_TIMEOUT)); - strUsage += HelpMessageOpt("-rpcwallet=", _("Send RPC for non-default wallet on RPC server (argument is wallet filename in pivxd directory, required if pivxd/-Qt runs with multiple wallets)")); + strUsage += HelpMessageOpt("-rpcwallet=", _("Send RPC for non-default wallet on RPC server (needs to exactly match corresponding -wallet option passed to pivxd)")); return strUsage; } diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 33aadc689c7f..39bcbae970ff 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -125,6 +125,7 @@ bool CDBEnv::Open(bool retry) boost::this_thread::interruption_point(); fs::path pathIn = strPath; + TryCreateDirectories(pathIn); if (!LockDirectory(pathIn, ".walletlock")) { LogPrintf("Cannot obtain a lock on wallet directory %s. Another instance of PIVX may be using it.\n", strPath); return false; diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 91718c55e60c..9ad15ccf677a 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -29,7 +29,7 @@ std::string GetWalletHelpString(bool showDebug) strUsage += HelpMessageOpt("-spendzeroconfchange", strprintf(_("Spend unconfirmed change when sending transactions (default: %u)"), DEFAULT_SPEND_ZEROCONF_CHANGE)); strUsage += HelpMessageOpt("-txconfirmtarget=", strprintf(_("If paytxfee is not set, include enough fee so transactions begin confirmation on average within n blocks (default: %u)"), 1)); strUsage += HelpMessageOpt("-upgradewallet", _("Upgrade wallet to latest format") + " " + _("on startup")); - strUsage += HelpMessageOpt("-wallet=", _("Specify wallet file (within data directory)") + " " + strprintf(_("(default: %s)"), DEFAULT_WALLET_DAT)); + strUsage += HelpMessageOpt("-wallet=", _("Specify wallet database path. Can be specified multiple times to load multiple wallets. Path is interpreted relative to if it is not absolute, and will be created if it does not exist.") + " " + strprintf(_("(default: %s)"), DEFAULT_WALLET_DAT)); strUsage += HelpMessageOpt("-walletdir=", _("Specify directory to hold wallets (default: /wallets if it exists, otherwise )")); strUsage += HelpMessageOpt("-walletnotify=", _("Execute command when a wallet transaction changes (%s in cmd is replaced by TxID)")); strUsage += HelpMessageOpt("-zapwallettxes=", _("Delete all wallet transactions and only recover those parts of the blockchain through -rescan on startup") + @@ -165,13 +165,6 @@ bool WalletVerify() std::set wallet_paths; for (const std::string& walletFile : gArgs.GetArgs("-wallet")) { - if (fs::path(walletFile).filename() != walletFile) { - return UIError(strprintf(_("Error loading wallet %s. %s parameter must only specify a filename (not a path)."), walletFile, "-wallet")); - } - if (SanitizeString(walletFile, SAFE_CHARS_FILENAME) != walletFile) { - return UIError(strprintf(_("Error loading wallet %s. Invalid characters in %s filename."), walletFile, "-wallet")); - } - fs::path wallet_path = fs::absolute(walletFile, GetWalletDir()); if (fs::exists(wallet_path) && (!fs::is_regular_file(wallet_path) || fs::is_symlink(wallet_path))) { diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 6fe76133a548..e25ca51fd088 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -19,7 +19,6 @@ class MultiWalletTest(PivxTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 - self.extra_args = [['-wallet=w1', '-wallet=w2', '-wallet=w3', '-wallet=w'], []] self.supports_cli = True def run_test(self): @@ -29,9 +28,28 @@ def run_test(self): wallet_dir = lambda *p: data_dir('wallets', *p) wallet = lambda name: node.get_wallet_rpc(name) - assert_equal(set(node.listwallets()), {"w1", "w2", "w3", "w"}) - + # check wallet.dat is created self.stop_nodes() + assert_equal(os.path.isfile(wallet_dir('wallet.dat')), True) + + # restart node with a mix of wallet names: + # w1, w2, w3 - to verify new wallets created when non-existing paths specified + # w - to verify wallet name matching works when one wallet path is prefix of another + # sub/w5 - to verify relative wallet path is created correctly + # extern/w6 - to verify absolute wallet path is created correctly + # wallet.dat - to verify existing wallet file is loaded correctly + wallet_names = ['w1', 'w2', 'w3', 'w', 'sub/w5', os.path.join(self.options.tmpdir, 'extern/w6'), 'wallet.dat'] + extra_args = ['-wallet={}'.format(n) for n in wallet_names] + self.start_node(0, extra_args) + assert_equal(set(node.listwallets()), set(wallet_names)) + + # check that all requested wallets were created + self.stop_node(0) + for wallet_name in wallet_names: + assert_equal(os.path.isfile(wallet_dir(wallet_name)), True) + + # should not initialize if wallet path can't be created + self.assert_start_raises_init_error(0, ['-wallet=wallet.dat/bad'], 'File exists') self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist') self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir()) @@ -93,15 +111,17 @@ def run_test(self): self.restart_node(0, ['-walletdir='+competing_wallet_dir]) self.assert_start_raises_init_error(1, ['-walletdir='+competing_wallet_dir], 'Error initializing wallet database environment') - self.restart_node(0, self.extra_args[0]) + self.restart_node(0, extra_args) - w1 = wallet("w1") - w2 = wallet("w2") - w3 = wallet("w3") - w4 = wallet("w") + wallets = [wallet(w) for w in wallet_names] wallet_bad = wallet("bad") - w1.generate(1) + # check wallet names and balances + wallets[0].generate(1) + for wallet_name, wallet in zip(wallet_names, wallets): + info = wallet.getwalletinfo() + assert_equal(info['immature_balance'], 250 if wallet is wallets[0] else 0) + assert_equal(info['walletname'], wallet_name) # accessing invalid wallet fails assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", wallet_bad.getwalletinfo) @@ -109,24 +129,7 @@ def run_test(self): # accessing wallet RPC without using wallet endpoint fails assert_raises_rpc_error(-19, "Wallet file not specified", node.getwalletinfo) - # check w1 wallet balance - w1_info = w1.getwalletinfo() - assert_equal(w1_info['immature_balance'], 250) - w1_name = w1_info['walletname'] - assert_equal(w1_name, "w1") - - # check w2 wallet balance - w2_info = w2.getwalletinfo() - assert_equal(w2_info['immature_balance'], 0) - w2_name = w2_info['walletname'] - assert_equal(w2_name, "w2") - - w3_name = w3.getwalletinfo()['walletname'] - assert_equal(w3_name, "w3") - - w4_name = w4.getwalletinfo()['walletname'] - assert_equal(w4_name, "w") - + w1, w2, w3, w4, *_ = wallets w1.generate(101) assert_equal(w1.getbalance(), 500) assert_equal(w2.getbalance(), 0) From c2d3a07b3b6f3ccc4a11c632e6230582f7dcf5e8 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Wed, 15 Nov 2017 15:44:36 -0500 Subject: [PATCH 17/32] Create new wallet databases as directories rather than files This change should make it easier for users to make complete backups of wallets because they can now just back up the specified `-wallet=` path directly, instead of having to back up the specified path as well as the transaction log directory (for incompletely flushed wallets). Another advantage of this change is that if two wallets are located in the same directory, they will now use their own BerkeleyDB environments instead using a shared environment. Using a shared environment makes it difficult to manage and back up wallets separately because transaction log files will contain a mix of data from all wallets in the environment. --- doc/release-notes.md | 29 +++++++++++++++++--- src/init.cpp | 9 +++---- src/pivx-cli.cpp | 4 +-- src/wallet/db.cpp | 15 +++++++++-- src/wallet/init.cpp | 22 +++++++++++---- src/wallet/wallet.cpp | 2 -- src/wallet/wallet.h | 1 - test/functional/feature_config_args.py | 9 +++---- test/functional/wallet_multiwallet.py | 37 +++++++++++++++++--------- 9 files changed, 88 insertions(+), 40 deletions(-) diff --git a/doc/release-notes.md b/doc/release-notes.md index 04dc8f743da4..210c5564afc2 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -324,10 +324,31 @@ External wallet files --------------------- The `-wallet=` option now accepts full paths instead of requiring wallets -to be located in the -walletdir directory. When wallets are located in -different directories, wallet data will be stored independently, so data from -every wallet is not mixed into the same /database/log.?????????? -files. +to be located in the -walletdir directory. + +Newly created wallet format +--------------------------- + +If `-wallet=` is specified with a path that does not exist, it will now +create a wallet directory at the specified location (containing a wallet.dat +data file, a db.log file, and database/log.?????????? files) instead of just +creating a data file at the path and storing log files in the parent +directory. This should make backing up wallets more straightforward than +before because the specified wallet path can just be directly archived without +having to look in the parent directory for transaction log files. + +For backwards compatibility, wallet paths that are names of existing data files +in the `-walletdir` directory will continue to be accepted and interpreted the +same as before. + +Low-level RPC changes +--------------------- + +- When PIVX is not started with any `-wallet=` options, the name of + the default wallet returned by `getwalletinfo` and `listwallets` RPCs is + now the empty string `""` instead of `"wallet.dat"`. If PIVX is started + with any `-wallet=` options, there is no change in behavior, and the + name of any wallet is just its `` string. Database cache memory increased -------------------------------- diff --git a/src/init.cpp b/src/init.cpp index 93730bdb7624..92331c347917 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -847,8 +847,6 @@ namespace { // Variables internal to initialization process only int nUserMaxConnections; int nFD; ServiceFlags nLocalServices = NODE_NETWORK; - - std::string strWalletFile; } bool AppInitBasicSetup() @@ -1173,7 +1171,6 @@ bool AppInitParameterInteraction() } #ifdef ENABLE_WALLET - strWalletFile = gArgs.GetArg("-wallet", DEFAULT_WALLET_DAT); if (!WalletParameterInteraction()) return false; #endif // ENABLE_WALLET @@ -1258,9 +1255,9 @@ bool AppInitMain() // Warn about relative -datadir path. if (gArgs.IsArgSet("-datadir") && !fs::path(gArgs.GetArg("-datadir", "")).is_absolute()) { LogPrintf("Warning: relative datadir option '%s' specified, which will be interpreted relative to the " - "current working directory '%s'. This is fragile, because if bitcoin is started in the future " - "from a different location, it will be unable to locate the current data files. There could " - "also be data loss if bitcoin is started while in a temporary directory.\n", + "current working directory '%s'. This is fragile because if PIVX is started in the future " + "from a different location. It will be unable to locate the current data files. There could " + "also be data loss if PIVX is started while in a temporary directory.\n", gArgs.GetArg("-datadir", ""), fs::current_path().string()); } diff --git a/src/pivx-cli.cpp b/src/pivx-cli.cpp index a3eb2c9298e5..5f22ab957583 100644 --- a/src/pivx-cli.cpp +++ b/src/pivx-cli.cpp @@ -193,8 +193,8 @@ UniValue CallRPC(const std::string& strMethod, const UniValue& params) // check if we should use a special wallet endpoint std::string endpoint = "/"; - std::string walletName = gArgs.GetArg("-rpcwallet", ""); - if (!walletName.empty()) { + if (!gArgs.GetArgs("-rpcwallet").empty()) { + std::string walletName = gArgs.GetArg("-rpcwallet", ""); char* encodedURI = evhttp_uriencode(walletName.c_str(), walletName.size(), false); if (encodedURI) { endpoint = "/wallet/"+ std::string(encodedURI); diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 39bcbae970ff..e715a45bb28b 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -62,8 +62,19 @@ std::map g_dbenvs; //!< Map from directory name to open db CDBEnv* GetWalletEnv(const fs::path& wallet_path, std::string& database_filename) { - fs::path env_directory = wallet_path.parent_path(); - database_filename = wallet_path.filename().string(); + fs::path env_directory; + if (fs::is_regular_file(wallet_path)) { + // Special case for backwards compatibility: if wallet path points to an + // existing file, treat it as the path to a BDB data file in a parent + // directory that also contains BDB log files. + env_directory = wallet_path.parent_path(); + database_filename = wallet_path.filename().string(); + } else { + // Normal case: Interpret wallet path as a directory path containing + // data and log files. + env_directory = wallet_path; + database_filename = "wallet.dat"; + } LOCK(cs_db); // Note: An ununsed temporary CDBEnv object may be created inside the // emplace function if the key already exists. This is a little inefficient, diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 9ad15ccf677a..00f3efa45c7a 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -29,7 +29,7 @@ std::string GetWalletHelpString(bool showDebug) strUsage += HelpMessageOpt("-spendzeroconfchange", strprintf(_("Spend unconfirmed change when sending transactions (default: %u)"), DEFAULT_SPEND_ZEROCONF_CHANGE)); strUsage += HelpMessageOpt("-txconfirmtarget=", strprintf(_("If paytxfee is not set, include enough fee so transactions begin confirmation on average within n blocks (default: %u)"), 1)); strUsage += HelpMessageOpt("-upgradewallet", _("Upgrade wallet to latest format") + " " + _("on startup")); - strUsage += HelpMessageOpt("-wallet=", _("Specify wallet database path. Can be specified multiple times to load multiple wallets. Path is interpreted relative to if it is not absolute, and will be created if it does not exist.") + " " + strprintf(_("(default: %s)"), DEFAULT_WALLET_DAT)); + strUsage += HelpMessageOpt("-wallet=", _("Specify wallet database path. Can be specified multiple times to load multiple wallets. Path is interpreted relative to if it is not absolute, and will be created if it does not exist (as a directory containing a wallet.dat file and log files). For backwards compatibility this will also accept names of existing data files in .)")); strUsage += HelpMessageOpt("-walletdir=", _("Specify directory to hold wallets (default: /wallets if it exists, otherwise )")); strUsage += HelpMessageOpt("-walletnotify=", _("Execute command when a wallet transaction changes (%s in cmd is replaced by TxID)")); strUsage += HelpMessageOpt("-zapwallettxes=", _("Delete all wallet transactions and only recover those parts of the blockchain through -rescan on startup") + @@ -60,7 +60,7 @@ bool WalletParameterInteraction() return UIError(strprintf(_("%s is not allowed in combination with enabled wallet functionality"), "-sysperms")); } - gArgs.SoftSetArg("-wallet", DEFAULT_WALLET_DAT); + gArgs.SoftSetArg("-wallet", ""); const bool is_multiwallet = gArgs.GetArgs("-wallet").size() > 1; if (gArgs.GetBoolArg("-salvagewallet", false)) { @@ -165,10 +165,22 @@ bool WalletVerify() std::set wallet_paths; for (const std::string& walletFile : gArgs.GetArgs("-wallet")) { + // Do some checking on wallet path. It should be either a: + // + // 1. Path where a directory can be created. + // 2. Path to an existing directory. + // 3. Path to a symlink to a directory. + // 4. For backwards compatibility, the name of a data file in -walletdir. fs::path wallet_path = fs::absolute(walletFile, GetWalletDir()); - - if (fs::exists(wallet_path) && (!fs::is_regular_file(wallet_path) || fs::is_symlink(wallet_path))) { - return UIError(strprintf(_("Error loading wallet %s. %s filename must be a regular file."), walletFile, "-wallet")); + fs::file_type path_type = fs::symlink_status(wallet_path).type(); + if (!(path_type == fs::file_not_found || path_type == fs::directory_file || + (path_type == fs::symlink_file && fs::is_directory(wallet_path)) || + (path_type == fs::regular_file && fs::path(walletFile).filename() == walletFile))) { + return UIError(strprintf( + _("Invalid -wallet path '%s'. -wallet path should point to a directory where wallet.dat and " + "database/log.?????????? files can be stored, a location where such a directory could be created " + "or (for backwards compatibility) the name of an existing data file in -walletdir (%s)"), + walletFile, GetWalletDir())); } if (!wallet_paths.insert(wallet_path).second) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index dd6ae5da16e5..078bafbb8bdc 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -39,8 +39,6 @@ bool bdisableSystemnotifications = false; // Those bubbles can be annoying and s bool fPayAtLeastCustomFee = true; bool bSpendZeroConfChange = DEFAULT_SPEND_ZEROCONF_CHANGE; -const char * DEFAULT_WALLET_DAT = "wallet.dat"; - /** * Fees smaller than this (in upiv) are considered zero fee (for transaction creation) * We are ~100 times smaller then bitcoin now (2015-06-23), set minTxFee 10 times higher diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 32a9f7b78fee..659e8eee2b13 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -81,7 +81,6 @@ static const unsigned int DEFAULT_CREATEWALLETBACKUPS = 10; //! Default for -disablewallet static const bool DEFAULT_DISABLE_WALLET = false; -extern const char * DEFAULT_WALLET_DAT; static const int64_t TIMESTAMP_MIN = 0; class CAddressBookIterator; diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index e66a1cd34447..df0b890dba88 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -42,16 +42,15 @@ def run_test(self): # Create the directory and ensure the config file now works os.mkdir(new_data_dir) - # Temporarily disabled, because this test would access the user's home dir (~/.pivx) - #self.start_node(0, ['-conf='+conf_file, '-wallet=w1']) - #self.stop_node(0) - #assert os.path.isfile(os.path.join(new_data_dir, 'regtest', 'wallets', 'w1')) + self.start_node(0, ['-conf='+conf_file, '-wallet=w1']) + self.stop_node(0) + assert os.path.exists(os.path.join(new_data_dir, 'regtest', 'wallets', 'w1')) # Ensure command line argument overrides datadir in conf os.mkdir(new_data_dir_2) self.nodes[0].datadir = new_data_dir_2 self.start_node(0, ['-datadir='+new_data_dir_2, '-conf='+conf_file, '-wallet=w2']) - assert os.path.isfile(os.path.join(new_data_dir_2, 'regtest', 'wallets', 'w2')) + assert os.path.exists(os.path.join(new_data_dir_2, 'regtest', 'wallets', 'w2')) if __name__ == '__main__': ConfArgsTest().main() diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index e25ca51fd088..52a0abc4a2af 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -32,13 +32,24 @@ def run_test(self): self.stop_nodes() assert_equal(os.path.isfile(wallet_dir('wallet.dat')), True) + # create symlink to verify wallet directory path can be referenced + # through symlink + os.mkdir(wallet_dir('w7')) + os.symlink('w7', wallet_dir('w7_symlink')) + + # rename wallet.dat to make sure plain wallet file paths (as opposed to + # directory paths) can be loaded + os.rename(wallet_dir("wallet.dat"), wallet_dir("w8")) + # restart node with a mix of wallet names: # w1, w2, w3 - to verify new wallets created when non-existing paths specified # w - to verify wallet name matching works when one wallet path is prefix of another # sub/w5 - to verify relative wallet path is created correctly # extern/w6 - to verify absolute wallet path is created correctly - # wallet.dat - to verify existing wallet file is loaded correctly - wallet_names = ['w1', 'w2', 'w3', 'w', 'sub/w5', os.path.join(self.options.tmpdir, 'extern/w6'), 'wallet.dat'] + # w7_symlink - to verify symlinked wallet path is initialized correctly + # w8 - to verify existing wallet file is loaded correctly + # '' - to verify default wallet file is created correctly + wallet_names = ['w1', 'w2', 'w3', 'w', 'sub/w5', os.path.join(self.options.tmpdir, 'extern/w6'), 'w7_symlink', 'w8', ''] extra_args = ['-wallet={}'.format(n) for n in wallet_names] self.start_node(0, extra_args) assert_equal(set(node.listwallets()), set(wallet_names)) @@ -46,10 +57,13 @@ def run_test(self): # check that all requested wallets were created self.stop_node(0) for wallet_name in wallet_names: - assert_equal(os.path.isfile(wallet_dir(wallet_name)), True) + if os.path.isdir(wallet_dir(wallet_name)): + assert_equal(os.path.isfile(wallet_dir(wallet_name, "wallet.dat")), True) + else: + assert_equal(os.path.isfile(wallet_dir(wallet_name)), True) # should not initialize if wallet path can't be created - self.assert_start_raises_init_error(0, ['-wallet=wallet.dat/bad'], 'File exists') + self.assert_start_raises_init_error(0, ['-wallet=wallet.dat/bad'], 'Not a directory') self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist') self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir()) @@ -58,17 +72,14 @@ def run_test(self): # should not initialize if there are duplicate wallets self.assert_start_raises_init_error(0, ['-wallet=w1', '-wallet=w1'], 'Error loading wallet w1. Duplicate -wallet filename specified.') - # should not initialize if wallet file is a directory - os.mkdir(wallet_dir('w11')) - self.assert_start_raises_init_error(0, ['-wallet=w11'], 'Error loading wallet w11. -wallet filename must be a regular file.') - - #should not initialize if one wallet is a copy of another - shutil.copyfile(wallet_dir('w2'), wallet_dir('w22')) # !TODO: backport bitcoin#11970 - self.assert_start_raises_init_error(0, ['-wallet=w2', '-wallet=w22'], 'duplicates fileid') + # !TODO: backport bitcoin#11970 + # should not initialize if one wallet is a copy of another + shutil.copyfile(wallet_dir('w8'), wallet_dir('w8_copy')) + self.assert_start_raises_init_error(0, ['-wallet=w8', '-wallet=w8_copy'], 'duplicates fileid') # should not initialize if wallet file is a symlink - os.symlink(wallet_dir('w1'), wallet_dir('w12')) # !TODO: backport bitcoin#11970 - self.assert_start_raises_init_error(0, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.') + os.symlink('w8', wallet_dir('w8_symlink')) + self.assert_start_raises_init_error(0, ['-wallet=w8_symlink'], 'Invalid -wallet path') # should not initialize if the specified walletdir does not exist self.assert_start_raises_init_error(0, ['-walletdir=bad'], 'Error: Specified -walletdir "bad" does not exist') From 23458caf8dfdfd49d805962330ffc813dbb755fd Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 15 Jun 2021 18:32:56 -0300 Subject: [PATCH 18/32] GUI: Implement and connect WalletModel::getWalletPath(). --- .../pivx/settings/settingsinformationwidget.cpp | 8 +++++++- .../pivx/settings/settingsinformationwidget.h | 1 + src/qt/pivx/settings/settingswidget.cpp | 2 +- src/qt/rpcconsole.cpp | 17 +++++++++++++---- src/qt/rpcconsole.h | 3 +++ src/qt/walletmodel.cpp | 5 +++++ src/qt/walletmodel.h | 3 +++ 7 files changed, 33 insertions(+), 6 deletions(-) diff --git a/src/qt/pivx/settings/settingsinformationwidget.cpp b/src/qt/pivx/settings/settingsinformationwidget.cpp index 39fbf03ad7b1..90309f9bf2fb 100644 --- a/src/qt/pivx/settings/settingsinformationwidget.cpp +++ b/src/qt/pivx/settings/settingsinformationwidget.cpp @@ -91,7 +91,6 @@ SettingsInformationWidget::SettingsInformationWidget(PIVXGUI* _window,QWidget *p #ifdef ENABLE_WALLET // Wallet data -- remove it with if it's needed ui->labelInfoBerkeley->setText(DbEnv::version(0, 0, 0)); - ui->labelInfoDataDir->setText(QString::fromStdString(GetDataDir().string() + QDir::separator().toLatin1() + gArgs.GetArg("-wallet", DEFAULT_WALLET_DAT))); #else ui->labelInfoBerkeley->setText(tr("No information")); #endif @@ -127,6 +126,13 @@ void SettingsInformationWidget::loadClientModel() } } +void SettingsInformationWidget::loadWalletModel() +{ + if (walletModel) { + ui->labelInfoDataDir->setText(walletModel->getWalletPath()); + } +} + void SettingsInformationWidget::setNumConnections(int count) { if (!clientModel) diff --git a/src/qt/pivx/settings/settingsinformationwidget.h b/src/qt/pivx/settings/settingsinformationwidget.h index c29caf8aed13..5737aaededb3 100644 --- a/src/qt/pivx/settings/settingsinformationwidget.h +++ b/src/qt/pivx/settings/settingsinformationwidget.h @@ -22,6 +22,7 @@ class SettingsInformationWidget : public PWidget ~SettingsInformationWidget() override; void loadClientModel() override; + void loadWalletModel() override; void run(int type) override; void onError(QString error, int type) override; diff --git a/src/qt/pivx/settings/settingswidget.cpp b/src/qt/pivx/settings/settingswidget.cpp index 5db04f16da5b..3977d75c7ffc 100644 --- a/src/qt/pivx/settings/settingswidget.cpp +++ b/src/qt/pivx/settings/settingswidget.cpp @@ -9,7 +9,6 @@ #include "optionsmodel.h" #include "clientmodel.h" #include "utilitydialog.h" -#include "wallet/wallet.h" #include #include @@ -202,6 +201,7 @@ void SettingsWidget::loadWalletModel() this->settingsBitToolWidget->setWalletModel(this->walletModel); this->settingsDisplayOptionsWidget->setWalletModel(this->walletModel); this->settingsWalletOptionsWidget->setWalletModel(this->walletModel); + this->settingsInformationWidget->setWalletModel(this->walletModel); } void SettingsWidget::onResetAction() diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index a6057e6189e2..dc8f99eccba3 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -12,6 +12,7 @@ #include "guiutil.h" #include "peertablemodel.h" #include "qt/rpcexecutor.h" +#include "walletmodel.h" #include "chainparams.h" #include "netbase.h" @@ -62,11 +63,12 @@ const struct { RPCConsole::RPCConsole(QWidget* parent) : QDialog(parent, Qt::WindowSystemMenuHint | Qt::WindowTitleHint | Qt::WindowCloseButtonHint), ui(new Ui::RPCConsole), - clientModel(0), + clientModel(nullptr), + walletModel(nullptr), historyPtr(0), cachedNodeid(-1), - peersTableContextMenu(0), - banTableContextMenu(0) + peersTableContextMenu(nullptr), + banTableContextMenu(nullptr) { ui->setupUi(this); GUIUtil::restoreWindowGeometry("nRPCConsoleWindow", this->size(), this); @@ -94,7 +96,6 @@ RPCConsole::RPCConsole(QWidget* parent) : QDialog(parent, Qt::WindowSystemMenuHi // set library version labels #ifdef ENABLE_WALLET ui->berkeleyDBVersion->setText(DbEnv::version(0, 0, 0)); - ui->wallet_path->setText(QString::fromStdString(GetDataDir().string() + QDir::separator().toLatin1() + gArgs.GetArg("-wallet", DEFAULT_WALLET_DAT))); #else ui->label_berkeleyDBVersion->hide(); @@ -287,6 +288,14 @@ void RPCConsole::setClientModel(ClientModel* model) } } +void RPCConsole::setWalletModel(WalletModel* model) +{ + walletModel = model; + if (walletModel) { + ui->wallet_path->setText(walletModel->getWalletPath()); + } +} + /** Restart wallet with "-salvagewallet" */ void RPCConsole::walletSalvage() { diff --git a/src/qt/rpcconsole.h b/src/qt/rpcconsole.h index cc7a7ad8b5f5..d3f7bb2284e8 100644 --- a/src/qt/rpcconsole.h +++ b/src/qt/rpcconsole.h @@ -16,6 +16,7 @@ class ClientModel; class RPCTimerInterface; +class WalletModel; namespace Ui { @@ -37,6 +38,7 @@ class RPCConsole : public QDialog ~RPCConsole(); void setClientModel(ClientModel* model); + void setWalletModel(WalletModel* model); protected: virtual bool eventFilter(QObject* obj, QEvent* event); @@ -140,6 +142,7 @@ public Q_SLOTS: Ui::RPCConsole* ui; ClientModel* clientModel; + WalletModel* walletModel; QStringList history; int historyPtr; NodeId cachedNodeid; diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 034cfc89befd..13fdbb5ecd0e 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -120,6 +120,11 @@ bool WalletModel::upgradeWallet(std::string& upgradeError) return wallet->Upgrade(upgradeError, prev_version); } +QString WalletModel::getWalletPath() +{ + return QString::fromStdString(GetDataDir().string() + QDir::separator().toLatin1() + wallet->GetName()); +} + CAmount WalletModel::getBalance(const CCoinControl* coinControl, bool fIncludeDelegated, bool fUnlockedOnly, bool fIncludeShielded) const { if (coinControl) { diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 1fa09393584b..0ad630a6ec72 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -161,6 +161,9 @@ class WalletModel : public QObject bool isSaplingWalletEnabled() const; bool upgradeWallet(std::string& upgradeError); + // Returns the path to the first wallet db (future: add multi-wallet support) + QString getWalletPath(); + interfaces::WalletBalances GetWalletBalances() { return m_cached_balances; }; CAmount getBalance(const CCoinControl* coinControl = nullptr, bool fIncludeDelegated = true, bool fUnlockedOnly = false, bool fIncludeShielded = true) const; From d9e1c6b56def5a6ea9903c244f70ceae40b9ce0c Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 15 Jun 2021 18:55:34 -0300 Subject: [PATCH 19/32] Abstract VerifyWalletPath and connect it to init and GUI. --- src/qt/pivx.cpp | 15 +++++---------- src/wallet/init.cpp | 20 +++----------------- src/wallet/walletutil.cpp | 22 ++++++++++++++++++++++ src/wallet/walletutil.h | 3 +++ 4 files changed, 33 insertions(+), 27 deletions(-) diff --git a/src/qt/pivx.cpp b/src/qt/pivx.cpp index ccd933a1d4e5..15c38ab1c2bd 100644 --- a/src/qt/pivx.cpp +++ b/src/qt/pivx.cpp @@ -683,17 +683,12 @@ int main(int argc, char* argv[]) bool ret = true; #ifdef ENABLE_WALLET - // Check if the wallet exists or need to be created - std::string strWalletFile = gArgs.GetArg("-wallet", DEFAULT_WALLET_DAT); - std::string strDataDir = GetDataDir().string(); - // Wallet file must be a plain filename without a directory - fs::path wallet_file_path(strWalletFile); - if (strWalletFile != wallet_file_path.filename().string()) { - throw std::runtime_error(strprintf(_("Wallet %s resides outside data directory %s"), strWalletFile, strDataDir)); - } + // Check if at least one wallet exists or needs to be created + const std::string& strWalletFile = gArgs.GetArg("-wallet", ""); + auto opRes = VerifyWalletPath(strWalletFile); + if (!opRes) throw std::runtime_error(opRes.getError()); - fs::path pathBootstrap = GetDataDir() / strWalletFile; - if (!fs::exists(pathBootstrap)) { + if (!fs::exists(fs::absolute(strWalletFile, GetWalletDir()))) { // wallet doesn't exist, popup tutorial screen. ret = app.createTutorialScreen(); } diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 00f3efa45c7a..c2ac22ec133e 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -165,24 +165,10 @@ bool WalletVerify() std::set wallet_paths; for (const std::string& walletFile : gArgs.GetArgs("-wallet")) { - // Do some checking on wallet path. It should be either a: - // - // 1. Path where a directory can be created. - // 2. Path to an existing directory. - // 3. Path to a symlink to a directory. - // 4. For backwards compatibility, the name of a data file in -walletdir. - fs::path wallet_path = fs::absolute(walletFile, GetWalletDir()); - fs::file_type path_type = fs::symlink_status(wallet_path).type(); - if (!(path_type == fs::file_not_found || path_type == fs::directory_file || - (path_type == fs::symlink_file && fs::is_directory(wallet_path)) || - (path_type == fs::regular_file && fs::path(walletFile).filename() == walletFile))) { - return UIError(strprintf( - _("Invalid -wallet path '%s'. -wallet path should point to a directory where wallet.dat and " - "database/log.?????????? files can be stored, a location where such a directory could be created " - "or (for backwards compatibility) the name of an existing data file in -walletdir (%s)"), - walletFile, GetWalletDir())); - } + auto opRes = VerifyWalletPath(walletFile); + if (!opRes) return UIError(opRes.getError()); + fs::path wallet_path = fs::absolute(walletFile, GetWalletDir()); if (!wallet_paths.insert(wallet_path).second) { return UIError(strprintf(_("Error loading wallet %s. Duplicate %s filename specified."), walletFile, "-wallet")); } diff --git a/src/wallet/walletutil.cpp b/src/wallet/walletutil.cpp index 6bccc7182d98..2d48453f15ac 100644 --- a/src/wallet/walletutil.cpp +++ b/src/wallet/walletutil.cpp @@ -26,3 +26,25 @@ fs::path GetWalletDir() return path; } + +OperationResult VerifyWalletPath(const std::string& walletFile) +{ + // Do some checking on wallet path. It should be either a: + // + // 1. Path where a directory can be created. + // 2. Path to an existing directory. + // 3. Path to a symlink to a directory. + // 4. For backwards compatibility, the name of a data file in -walletdir. + fs::path wallet_path = fs::absolute(walletFile, GetWalletDir()); + fs::file_type path_type = fs::symlink_status(wallet_path).type(); + if (!(path_type == fs::file_not_found || path_type == fs::directory_file || + (path_type == fs::symlink_file && fs::is_directory(wallet_path)) || + (path_type == fs::regular_file && fs::path(walletFile).filename() == walletFile))) { + return {false, (strprintf( + _("Invalid -wallet path '%s'. -wallet path should point to a directory where wallet.dat and " + "database/log.?????????? files can be stored, a location where such a directory could be created " + "or (for backwards compatibility) the name of an existing data file in -walletdir (%s)"), + walletFile, GetWalletDir()))}; + } + return {true}; +} diff --git a/src/wallet/walletutil.h b/src/wallet/walletutil.h index b1a232f494df..91c824ae708a 100644 --- a/src/wallet/walletutil.h +++ b/src/wallet/walletutil.h @@ -6,8 +6,11 @@ #define BITCOIN_WALLET_UTIL_H #include "fs.h" +#include "operationresult.h" //! Get the path of the wallet directory. fs::path GetWalletDir(); +//! Verify the wallet db's path +OperationResult VerifyWalletPath(const std::string& walletFile); #endif // BITCOIN_WALLET_UTIL_H From d86cd4f511e893ed88cb423f3a1768e51b039801 Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 15 Jun 2021 20:07:19 -0300 Subject: [PATCH 20/32] wallet: Add missing check for backup to source wallet file. --- src/wallet/db.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index e715a45bb28b..c547df0e3845 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -767,6 +767,11 @@ bool CWalletDBWrapper::Backup(const std::string& strDest) pathDest /= strFile; try { + if (fs::equivalent(pathSrc, pathDest)) { + LogPrintf("cannot backup to wallet source file %s\n", pathDest.string()); + return false; + } + #if BOOST_VERSION >= 107400 fs::copy_file(pathSrc.c_str(), pathDest, fs::copy_options::overwrite_existing); #elif BOOST_VERSION >= 105800 /* BOOST_LIB_VERSION 1_58 */ From 9ae619a0aaa18d3c439dc58c11539324cfb7787c Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 22 Jun 2021 13:53:08 -0300 Subject: [PATCH 21/32] Test: Use specific testing setups for wallet_zkeys_tests tests --- src/test/librust/wallet_zkeys_tests.cpp | 58 +++++++++++++------------ src/wallet/db.cpp | 6 +++ src/wallet/db.h | 4 ++ src/wallet/wallet.h | 6 +-- 4 files changed, 43 insertions(+), 31 deletions(-) diff --git a/src/test/librust/wallet_zkeys_tests.cpp b/src/test/librust/wallet_zkeys_tests.cpp index af5ff99fb78c..d846ff39fc63 100644 --- a/src/test/librust/wallet_zkeys_tests.cpp +++ b/src/test/librust/wallet_zkeys_tests.cpp @@ -21,7 +21,7 @@ * LoadZKeyMetadata() */ -BOOST_FIXTURE_TEST_SUITE(wallet_zkeys_tests, WalletTestingSetup) +BOOST_AUTO_TEST_SUITE(wallet_zkeys_tests) /** * This test covers Sapling methods on CWallet @@ -30,9 +30,7 @@ BOOST_FIXTURE_TEST_SUITE(wallet_zkeys_tests, WalletTestingSetup) * LoadSaplingZKey() * LoadSaplingZKeyMetadata() */ -BOOST_AUTO_TEST_CASE(StoreAndLoadSaplingZkeys) { - SelectParams(CBaseChainParams::MAIN); - +BOOST_FIXTURE_TEST_CASE(StoreAndLoadSaplingZkeys, TestingSetup) { CWallet wallet("dummy", CWalletDBWrapper::CreateDummy()); LOCK(wallet.cs_wallet); // wallet should be empty @@ -133,52 +131,54 @@ BOOST_AUTO_TEST_CASE(StoreAndLoadSaplingZkeys) { /** * This test covers methods on CWalletDB to load/save crypted sapling z keys. */ -BOOST_AUTO_TEST_CASE(WriteCryptedSaplingZkeyDirectToDb) { - SelectParams(CBaseChainParams::TESTNET); - +BOOST_FIXTURE_TEST_CASE(WriteCryptedSaplingZkeyDirectToDb, BasicTestingSetup) { fs::path path = fs::absolute("testWallet1", GetWalletDir()); - CWallet testWallet("testWallet1", CWalletDBWrapper::Create(path)); + path.make_preferred(); + std::unique_ptr testWallet = std::make_unique("testWallet1", CWalletDBWrapper::Create(path)); bool fFirstRun; - BOOST_CHECK_EQUAL(testWallet.LoadWallet(fFirstRun), DB_LOAD_OK); - BOOST_CHECK(!testWallet.HasSaplingSPKM()); - assert(testWallet.SetupSPKM(true)); + BOOST_CHECK_EQUAL(testWallet->LoadWallet(fFirstRun), DB_LOAD_OK); + BOOST_CHECK(!testWallet->HasSaplingSPKM()); + assert(testWallet->SetupSPKM(true)); // wallet should be empty std::set addrs; - testWallet.GetSaplingPaymentAddresses(addrs); + testWallet->GetSaplingPaymentAddresses(addrs); BOOST_CHECK_EQUAL(0, addrs.size()); // Add random key to the wallet - auto address = testWallet.GenerateNewSaplingZKey(); + auto address = testWallet->GenerateNewSaplingZKey(); // wallet should have one key - testWallet.GetSaplingPaymentAddresses(addrs); + testWallet->GetSaplingPaymentAddresses(addrs); BOOST_CHECK_EQUAL(1, addrs.size()); // encrypt wallet SecureString strWalletPass; strWalletPass.reserve(100); strWalletPass = "hello"; - BOOST_CHECK(testWallet.EncryptWallet(strWalletPass)); + BOOST_CHECK(testWallet->EncryptWallet(strWalletPass)); // adding a new key will fail as the wallet is locked - BOOST_CHECK_THROW(testWallet.GenerateNewSaplingZKey(), std::runtime_error); + BOOST_CHECK_THROW(testWallet->GenerateNewSaplingZKey(), std::runtime_error); // unlock wallet and then add - testWallet.Unlock(strWalletPass); - libzcash::SaplingPaymentAddress address2 = testWallet.GenerateNewSaplingZKey(); + testWallet->Unlock(strWalletPass); + libzcash::SaplingPaymentAddress address2 = testWallet->GenerateNewSaplingZKey(); + + // Force db close + testWallet->Flush(true); + testWallet.reset(); // Create a new wallet from the existing wallet path fFirstRun = false; - CWallet wallet2("testWallet1", CWalletDBWrapper::Create(path)); - BOOST_CHECK_EQUAL(DB_LOAD_OK, wallet2.LoadWallet(fFirstRun)); + std::unique_ptr wallet2 = std::make_unique("testWallet1", CWalletDBWrapper::Create(path)); + BOOST_CHECK_EQUAL(DB_LOAD_OK, wallet2->LoadWallet(fFirstRun)); // Confirm it's not the same as the other wallet - BOOST_CHECK(&testWallet != &wallet2); - BOOST_CHECK(wallet2.HasSaplingSPKM()); + BOOST_CHECK(wallet2->HasSaplingSPKM()); // wallet should have two keys - wallet2.GetSaplingPaymentAddresses(addrs); + wallet2->GetSaplingPaymentAddresses(addrs); BOOST_CHECK_EQUAL(2, addrs.size()); //check we have entries for our payment addresses @@ -187,16 +187,20 @@ BOOST_AUTO_TEST_CASE(WriteCryptedSaplingZkeyDirectToDb) { // spending key is crypted, so we can't extract valid payment address libzcash::SaplingExtendedSpendingKey keyOut; - BOOST_CHECK(!wallet2.GetSaplingExtendedSpendingKey(address, keyOut)); + BOOST_CHECK(!wallet2->GetSaplingExtendedSpendingKey(address, keyOut)); // unlock wallet to get spending keys and verify payment addresses - wallet2.Unlock(strWalletPass); + wallet2->Unlock(strWalletPass); - BOOST_CHECK(wallet2.GetSaplingExtendedSpendingKey(address, keyOut)); + BOOST_CHECK(wallet2->GetSaplingExtendedSpendingKey(address, keyOut)); BOOST_CHECK(address == keyOut.DefaultAddress()); - BOOST_CHECK(wallet2.GetSaplingExtendedSpendingKey(address2, keyOut)); + BOOST_CHECK(wallet2->GetSaplingExtendedSpendingKey(address2, keyOut)); BOOST_CHECK(address2 == keyOut.DefaultAddress()); + + // Force db close + wallet2->Flush(true); + wallet2.reset(); } diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index c547df0e3845..eab260c89644 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -796,3 +796,9 @@ void CWalletDBWrapper::Flush(bool shutdown) env->Flush(shutdown); } } + +void CWalletDBWrapper::CloseAndReset() +{ + env->Close(); + env->Reset(); +} diff --git a/src/wallet/db.h b/src/wallet/db.h index db1b0dc40b2c..64445f4fd3ba 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -144,6 +144,10 @@ class CWalletDBWrapper */ void Flush(bool shutdown); + /** Close and reset. + */ + void CloseAndReset(); + void IncrementUpdateCounter(); std::atomic nUpdateCounter; unsigned int nLastSeen; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 659e8eee2b13..6293a8475b58 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -737,10 +737,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface /** Get database handle used by this wallet. Ideally this function would * not be necessary. */ - CWalletDBWrapper& GetDBHandle() const - { - return *dbw; - } + CWalletDBWrapper* GetDBHandlePtr() const { return dbw.get(); } + CWalletDBWrapper& GetDBHandle() const { return *dbw; } /** Get a name for this wallet for logging/debugging purposes. */ From 16b46513e34831ebbbed55b96376b2f2c843c9f4 Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Wed, 23 Jun 2021 16:03:13 -0300 Subject: [PATCH 22/32] util: Fix multiple use of LockDirectory This commit fixes problems with calling LockDirectory multiple times on the same directory, or from multiple threads. It also fixes the build on OpenBSD. --- src/util/system.cpp | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/util/system.cpp b/src/util/system.cpp index 193ef2e676e8..72fb7ba0ad83 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -107,18 +107,32 @@ bool CheckDiskSpace(const fs::path& dir, uint64_t additional_bytes) bool LockDirectory(const fs::path& directory, const std::string& lockfile_name, bool probe_only) { + // A map that contains all the currently held directory locks. After + // successful locking, these will be held here until the global + // destructor cleans them up and thus automatically unlocks them. + static std::map> locks; + // Protect the map with a mutex + static std::mutex cs; + std::lock_guard ulock(cs); fs::path pathLockFile = directory / lockfile_name; - FILE* file = fsbridge::fopen(pathLockFile, "a"); // empty lock file; created if it doesn't exist. + + // If a lock for this directory already exists in the map, don't try to re-lock it + if (locks.count(pathLockFile.string())) { + return true; + } + + // Create empty lock file if it doesn't exist. + FILE* file = fsbridge::fopen(pathLockFile, "a"); if (file) fclose(file); try { - static std::map locks; - boost::interprocess::file_lock& lock = locks.emplace(pathLockFile.string(), pathLockFile.string().c_str()).first->second; - if (!lock.try_lock()) { + auto lock = std::make_unique(pathLockFile.string().c_str()); + if (!lock->try_lock()) { return false; } - if (probe_only) { - lock.unlock(); + if (!probe_only) { + // Lock successful and we're not just probing, put it into the map + locks.emplace(pathLockFile.string(), std::move(lock)); } } catch (const boost::interprocess::interprocess_exception& e) { return error("Error while attempting to lock directory %s: %s", directory.string(), e.what()); From 4cae8dce2ef364026d90591666cd4f7fc5426f8c Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Wed, 23 Jun 2021 16:06:40 -0300 Subject: [PATCH 23/32] test: Add unit test for LockDirectory Add a unit test for LockDirectory, introduced in btc#11281. --- src/test/util_tests.cpp | 130 ++++++++++++++++++++++++++++++++++++++++ src/util/system.cpp | 27 ++++++--- src/util/system.h | 6 ++ 3 files changed, 154 insertions(+), 9 deletions(-) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 1c0937763c02..2b2f06ad87e3 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -16,6 +16,10 @@ #include #include +#ifndef WIN32 +#include +#include +#endif #include @@ -963,6 +967,132 @@ BOOST_AUTO_TEST_CASE(test_Capitalize) BOOST_CHECK_EQUAL(Capitalize("\x00\xfe\xff"), "\x00\xfe\xff"); } +static void TestOtherThread(fs::path dirname, std::string lockname, bool *result) +{ + *result = LockDirectory(dirname, lockname); +} + +#ifndef WIN32 // Cannot do this test on WIN32 due to lack of fork() +static constexpr char LockCommand = 'L'; +static constexpr char UnlockCommand = 'U'; +static constexpr char ExitCommand = 'X'; + +static void TestOtherProcess(fs::path dirname, std::string lockname, int fd) +{ + char ch; + int rv; + while (true) { + rv = read(fd, &ch, 1); // Wait for command + assert(rv == 1); + switch(ch) { + case LockCommand: + ch = LockDirectory(dirname, lockname); + rv = write(fd, &ch, 1); + assert(rv == 1); + break; + case UnlockCommand: + ReleaseDirectoryLocks(); + ch = true; // Always succeeds + rv = write(fd, &ch, 1); + break; + case ExitCommand: + close(fd); + exit(0); + default: + assert(0); + } + } +} +#endif + +BOOST_AUTO_TEST_CASE(test_LockDirectory) +{ + fs::path dirname = fs::temp_directory_path() / fs::unique_path(); + const std::string lockname = ".lock"; +#ifndef WIN32 + // Revert SIGCHLD to default, otherwise boost.test will catch and fail on + // it: there is BOOST_TEST_IGNORE_SIGCHLD but that only works when defined + // at build-time of the boost library + void (*old_handler)(int) = signal(SIGCHLD, SIG_DFL); + + // Fork another process for testing before creating the lock, so that we + // won't fork while holding the lock (which might be undefined, and is not + // relevant as test case as that is avoided with -daemonize). + int fd[2]; + BOOST_CHECK_EQUAL(socketpair(AF_UNIX, SOCK_STREAM, 0, fd), 0); + pid_t pid = fork(); + if (!pid) { + BOOST_CHECK_EQUAL(close(fd[1]), 0); // Child: close parent end + TestOtherProcess(dirname, lockname, fd[0]); + } + BOOST_CHECK_EQUAL(close(fd[0]), 0); // Parent: close child end +#endif + // Lock on non-existent directory should fail + BOOST_CHECK_EQUAL(LockDirectory(dirname, lockname), false); + + fs::create_directories(dirname); + + // Probing lock on new directory should succeed + BOOST_CHECK_EQUAL(LockDirectory(dirname, lockname, true), true); + + // Persistent lock on new directory should succeed + BOOST_CHECK_EQUAL(LockDirectory(dirname, lockname), true); + + // Another lock on the directory from the same thread should succeed + BOOST_CHECK_EQUAL(LockDirectory(dirname, lockname), true); + + // Another lock on the directory from a different thread within the same process should succeed + bool threadresult; + std::thread thr(TestOtherThread, dirname, lockname, &threadresult); + thr.join(); + BOOST_CHECK_EQUAL(threadresult, true); +#ifndef WIN32 + // Try to aquire lock in child process while we're holding it, this should fail. + char ch; + BOOST_CHECK_EQUAL(write(fd[1], &LockCommand, 1), 1); + BOOST_CHECK_EQUAL(read(fd[1], &ch, 1), 1); + BOOST_CHECK_EQUAL((bool)ch, false); + + // Give up our lock + ReleaseDirectoryLocks(); + // Probing lock from our side now should succeed, but not hold on to the lock. + BOOST_CHECK_EQUAL(LockDirectory(dirname, lockname, true), true); + + // Try to acquire the lock in the child process, this should be succesful. + BOOST_CHECK_EQUAL(write(fd[1], &LockCommand, 1), 1); + BOOST_CHECK_EQUAL(read(fd[1], &ch, 1), 1); + BOOST_CHECK_EQUAL((bool)ch, true); + + // When we try to probe the lock now, it should fail. + BOOST_CHECK_EQUAL(LockDirectory(dirname, lockname, true), false); + + // Unlock the lock in the child process + BOOST_CHECK_EQUAL(write(fd[1], &UnlockCommand, 1), 1); + BOOST_CHECK_EQUAL(read(fd[1], &ch, 1), 1); + BOOST_CHECK_EQUAL((bool)ch, true); + + // When we try to probe the lock now, it should succeed. + BOOST_CHECK_EQUAL(LockDirectory(dirname, lockname, true), true); + + // Re-lock the lock in the child process, then wait for it to exit, check + // successful return. After that, we check that exiting the process + // has released the lock as we would expect by probing it. + int processstatus; + BOOST_CHECK_EQUAL(write(fd[1], &LockCommand, 1), 1); + BOOST_CHECK_EQUAL(write(fd[1], &ExitCommand, 1), 1); + BOOST_CHECK_EQUAL(waitpid(pid, &processstatus, 0), pid); + BOOST_CHECK_EQUAL(processstatus, 0); + BOOST_CHECK_EQUAL(LockDirectory(dirname, lockname, true), true); + + // Restore SIGCHLD + signal(SIGCHLD, old_handler); + BOOST_CHECK_EQUAL(close(fd[1]), 0); // Close our side of the socketpair +#endif + // Clean up + ReleaseDirectoryLocks(); + fs::remove_all(dirname); +} + namespace { struct Tracker diff --git a/src/util/system.cpp b/src/util/system.cpp index 72fb7ba0ad83..793f5300e0c6 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -105,19 +105,22 @@ bool CheckDiskSpace(const fs::path& dir, uint64_t additional_bytes) return free_bytes_available >= min_disk_space + additional_bytes; } +/** A map that contains all the currently held directory locks. After + * successful locking, these will be held here until the global destructor + * cleans them up and thus automatically unlocks them, or ReleaseDirectoryLocks + * is called. + */ +static std::map> dir_locks; +/** Mutex to protect dir_locks. */ +static std::mutex cs_dir_locks; + bool LockDirectory(const fs::path& directory, const std::string& lockfile_name, bool probe_only) { - // A map that contains all the currently held directory locks. After - // successful locking, these will be held here until the global - // destructor cleans them up and thus automatically unlocks them. - static std::map> locks; - // Protect the map with a mutex - static std::mutex cs; - std::lock_guard ulock(cs); + std::lock_guard ulock(cs_dir_locks); fs::path pathLockFile = directory / lockfile_name; // If a lock for this directory already exists in the map, don't try to re-lock it - if (locks.count(pathLockFile.string())) { + if (dir_locks.count(pathLockFile.string())) { return true; } @@ -132,7 +135,7 @@ bool LockDirectory(const fs::path& directory, const std::string& lockfile_name, } if (!probe_only) { // Lock successful and we're not just probing, put it into the map - locks.emplace(pathLockFile.string(), std::move(lock)); + dir_locks.emplace(pathLockFile.string(), std::move(lock)); } } catch (const boost::interprocess::interprocess_exception& e) { return error("Error while attempting to lock directory %s: %s", directory.string(), e.what()); @@ -140,6 +143,12 @@ bool LockDirectory(const fs::path& directory, const std::string& lockfile_name, return true; } +void ReleaseDirectoryLocks() +{ + std::lock_guard ulock(cs_dir_locks); + dir_locks.clear(); +} + /** * Interpret a string argument as a boolean. * diff --git a/src/util/system.h b/src/util/system.h index 3a8f5cf0404b..d3df7310272b 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -89,6 +89,12 @@ void AllocateFileRange(FILE* file, unsigned int offset, unsigned int length); bool CheckDiskSpace(const fs::path& dir, uint64_t additional_bytes = 0); bool RenameOver(fs::path src, fs::path dest); bool LockDirectory(const fs::path& directory, const std::string& lockfile_name, bool probe_only=false); + +/** Release all directory locks. This is used for unit testing only, at runtime + * the global destructor will take care of the locks. + */ +void ReleaseDirectoryLocks(); + bool TryCreateDirectories(const fs::path& p); fs::path GetDefaultDataDir(); // The blocks directory is always net specific. From 565abcd3b55dfbf95be6e08d96a7fd4f48cf0aa2 Mon Sep 17 00:00:00 2001 From: furszy Date: Wed, 23 Jun 2021 21:18:32 -0300 Subject: [PATCH 24/32] db: fix db path not removed from the open db environments map. --- src/wallet/db.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index eab260c89644..5c26fa1ab70f 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -692,8 +692,10 @@ void CDBEnv::Flush(bool fShutdown) if (mapFileUseCount.empty()) { dbenv->log_archive(&listp, DB_ARCH_REMOVE); Close(); - if (!fMockDb) + if (!fMockDb) { fs::remove_all(fs::path(strPath) / "database"); + } + g_dbenvs.erase(strPath); } } } From 351d2c8e3ff40b0e7da5c68303c1f1300b5dd030 Mon Sep 17 00:00:00 2001 From: furszy Date: Wed, 23 Jun 2021 23:11:53 -0300 Subject: [PATCH 25/32] wallet_tests: mock wallet db. --- src/wallet/test/wallet_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 1eec0a5c9736..7fa369848239 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -568,7 +568,7 @@ BOOST_AUTO_TEST_CASE(cached_balances_tests) CAmount nCredit = 20 * COIN; // Setup wallet - CWallet wallet("testWallet1", CWalletDBWrapper::Create(fs::absolute("testWallet1", GetWalletDir()))); + CWallet wallet("testWallet1", CWalletDBWrapper::CreateMock()); bool fFirstRun; BOOST_CHECK_EQUAL(wallet.LoadWallet(fFirstRun), DB_LOAD_OK); LOCK2(cs_main, wallet.cs_wallet); From 7aa251d1311264aacf400f57bb25394bb4b1c856 Mon Sep 17 00:00:00 2001 From: Daniel Kraft Date: Sun, 15 Jul 2018 11:17:50 +0200 Subject: [PATCH 26/32] wallet: Fix backupwallet for multiwallets backupwallet was broken for multiwallets in their own directories (i.e. something like DATADIR/wallets/mywallet/wallet.dat). In this case, the backup would use DATADIR/wallets/wallet.dat as source file and not take the specific wallet's directory into account. This led to either an error during the backup (if the wrong source file was not present) or would silently back up the wrong wallet; especially the latter behaviour can be quite bad for users. --- src/wallet/db.cpp | 2 +- test/functional/wallet_multiwallet.py | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 5c26fa1ab70f..047df48fd4c7 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -763,7 +763,7 @@ bool CWalletDBWrapper::Backup(const std::string& strDest) env->mapFileUseCount.erase(strFile); // Copy wallet file - fs::path pathSrc = GetWalletDir() / strFile; + fs::path pathSrc = env->Directory() / strFile; fs::path pathDest(strDest); if (fs::is_directory(pathDest)) pathDest /= strFile; diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 52a0abc4a2af..304c0a8d5bb6 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -28,6 +28,11 @@ def run_test(self): wallet_dir = lambda *p: data_dir('wallets', *p) wallet = lambda name: node.get_wallet_rpc(name) + def wallet_file(name): + if os.path.isdir(wallet_dir(name)): + return wallet_dir(name, "wallet.dat") + return wallet_dir(name) + # check wallet.dat is created self.stop_nodes() assert_equal(os.path.isfile(wallet_dir('wallet.dat')), True) @@ -57,10 +62,7 @@ def run_test(self): # check that all requested wallets were created self.stop_node(0) for wallet_name in wallet_names: - if os.path.isdir(wallet_dir(wallet_name)): - assert_equal(os.path.isfile(wallet_dir(wallet_name, "wallet.dat")), True) - else: - assert_equal(os.path.isfile(wallet_dir(wallet_name)), True) + assert_equal(os.path.isfile(wallet_file(wallet_name)), True) # should not initialize if wallet path can't be created self.assert_start_raises_init_error(0, ['-wallet=wallet.dat/bad'], 'Not a directory') @@ -159,5 +161,6 @@ def run_test(self): assert_equal(batch[0]["result"]["chain"], "regtest") #assert_equal(batch[1]["result"]["walletname"], "w1") + if __name__ == '__main__': MultiWalletTest().main() From 12a1e3909c9cb66d6555841290d2c33042478ed6 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Wed, 21 Jul 2021 01:36:47 +0200 Subject: [PATCH 27/32] [BUG] Sanitize wallet name in GetUniqueWalletBackupName --- src/wallet/wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 078bafbb8bdc..ea7df0aa74b8 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4367,7 +4367,7 @@ bool CWalletTx::AcceptToMemoryPool(CValidationState& state) std::string CWallet::GetUniqueWalletBackupName() const { - return strprintf("%s%s", (!m_name.empty() ? m_name : "null"), FormatISO8601DateTimeForBackup(GetTime())); + return strprintf("%s%s", (!m_name.empty() ? SanitizeString(m_name, SAFE_CHARS_FILENAME) : "null"), FormatISO8601DateTimeForBackup(GetTime())); } CWallet::CWallet(std::string name, std::unique_ptr dbw) : m_name(std::move(name)), dbw(std::move(dbw)) From 91b112b535da912b3a7b653bf89e14d5f2432620 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Tue, 20 Jul 2021 17:34:38 +0200 Subject: [PATCH 28/32] [Refactor][Bug] Fix automatic backups, final code deduplication --- src/wallet/walletdb.cpp | 59 +++------------------------ src/wallet/walletdb.h | 9 +--- test/functional/wallet_multiwallet.py | 6 +-- 3 files changed, 10 insertions(+), 64 deletions(-) diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 24971e5a917f..b6de8972da9b 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -828,12 +828,6 @@ void MaybeCompactWalletDB() fOneThread = false; } -void NotifyBacked(const CWallet& wallet, bool fSuccess, std::string strMessage) -{ - LogPrintf("%s\n", strMessage); - wallet.NotifyWalletBacked(fSuccess, strMessage); -} - typedef std::multimap folder_set_t; static folder_set_t buildBackupsMapSortedByLastWrite(const std::string& strWalletFile, const fs::path& backupsDir) { @@ -873,10 +867,8 @@ static bool cleanWalletBackups(folder_set_t& folder_set, int nWalletBackups, std return true; } -bool AutoBackupWallet(const CWallet& wallet, std::string& strBackupWarning, std::string& strBackupError) +bool AutoBackupWallet(CWallet& wallet, std::string& strBackupWarning, std::string& strBackupError) { - std::string strWalletFile = wallet.GetName(); - strBackupWarning = strBackupError = ""; int nWalletBackups = std::max(0, std::min(10, (int)gArgs.GetArg("-createwalletbackups", DEFAULT_CREATEWALLETBACKUPS))); if (nWalletBackups == 0) { @@ -887,25 +879,17 @@ bool AutoBackupWallet(const CWallet& wallet, std::string& strBackupWarning, std: fs::path backupsDir = GetDataDir() / "backups"; backupsDir.make_preferred(); TryCreateDirectories(backupsDir); - - // Copy wallet file - fs::path sourceFile = GetWalletDir() / strWalletFile; - fs::path backupFile = backupsDir / (strWalletFile + FormatISO8601DateTimeForBackup(GetTime())); - sourceFile.make_preferred(); + std::string strWalletFile = wallet.GetUniqueWalletBackupName(); + fs::path backupFile = backupsDir / strWalletFile; backupFile.make_preferred(); if (fs::exists(backupFile)) { LogPrintf("%s\n", _("Failed to create backup, file already exists! This could happen if you restarted wallet in less than 60 seconds. You can continue if you are ok with this.")); return false; } - if (!fs::exists(sourceFile)) { - strBackupWarning = strprintf(_("Failed to create backup, wallet file not found, path %s"), sourceFile); - LogPrintf("%s\n", strBackupWarning); - return false; - } - // Try to backup - if (!AttemptBackupWallet(&wallet, sourceFile, backupFile)) { + if (!wallet.BackupWallet(backupFile.string())) { + strBackupError = "Failed to backup wallet"; return false; // error is logged internally } @@ -914,39 +898,6 @@ bool AutoBackupWallet(const CWallet& wallet, std::string& strBackupWarning, std: return cleanWalletBackups(folder_set, nWalletBackups, strBackupWarning); } -bool AttemptBackupWallet(const CWallet* wallet, const fs::path& pathSrc, const fs::path& pathDest) -{ - bool retStatus; - std::string strMessage; - try { - if (fs::equivalent(pathSrc, pathDest)) { - LogPrintf("cannot backup to wallet source file %s\n", pathDest.string()); - return false; - } -#if BOOST_VERSION >= 107400 - fs::copy_file(pathSrc.c_str(), pathDest, fs::copy_options::overwrite_existing); -#elif BOOST_VERSION >= 105800 /* BOOST_LIB_VERSION 1_58 */ - fs::copy_file(pathSrc.c_str(), pathDest, fs::copy_option::overwrite_if_exists); -#else - std::ifstream src(pathSrc.c_str(), std::ios::binary | std::ios::in); - std::ofstream dst(pathDest.c_str(), std::ios::binary | std::ios::out | std::ios::trunc); - dst << src.rdbuf(); - dst.flush(); - src.close(); - dst.close(); -#endif - strMessage = strprintf("copied %s to %s\n", pathSrc.string(), pathDest.string()); - LogPrintf("%s : %s\n", __func__, strMessage); - retStatus = true; - } catch (const fs::filesystem_error& e) { - retStatus = false; - strMessage = strprintf("%s\n", e.what()); - LogPrintf("%s : %s\n", __func__, strMessage); - } - if (wallet) NotifyBacked(*wallet, retStatus, strMessage); - return retStatus; -} - // // Try to (very carefully!) recover wallet file if there is a problem. // diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 1e88b537750b..f664bb1c71d4 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -231,13 +231,8 @@ class CWalletDB CWalletDBWrapper& m_dbw; }; -void NotifyBacked(const CWallet& wallet, bool fSuccess, std::string strMessage); -// If wallet is null, the NotifyBacked signal will not be broadcasted. -// todo: move NotifyBacked() signal to the caller side and/or decouple it from here in another function -bool AttemptBackupWallet(const CWallet* wallet, const fs::path& pathSrc, const fs::path& pathDest); - -//! Called during init: Automatic backups of wallet not running (just copying and renaming dat file) -bool AutoBackupWallet(const CWallet& wallet, std::string& strBackupWarning, std::string& strBackupError); +//! Called during init: Automatic backups +bool AutoBackupWallet(CWallet& wallet, std::string& strBackupWarning, std::string& strBackupError); //! Compacts BDB state so that wallet.dat is self-contained (if there are changes) void MaybeCompactWalletDB(); diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 304c0a8d5bb6..6bb349a59c37 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -74,10 +74,10 @@ def wallet_file(name): # should not initialize if there are duplicate wallets self.assert_start_raises_init_error(0, ['-wallet=w1', '-wallet=w1'], 'Error loading wallet w1. Duplicate -wallet filename specified.') - # !TODO: backport bitcoin#11970 + # !TODO: automatic backup changes the environment, fix in the next commit # should not initialize if one wallet is a copy of another - shutil.copyfile(wallet_dir('w8'), wallet_dir('w8_copy')) - self.assert_start_raises_init_error(0, ['-wallet=w8', '-wallet=w8_copy'], 'duplicates fileid') + # shutil.copyfile(wallet_dir('w8'), wallet_dir('w8_copy')) + # self.assert_start_raises_init_error(0, ['-wallet=w8', '-wallet=w8_copy'], 'duplicates fileid') # should not initialize if wallet file is a symlink os.symlink('w8', wallet_dir('w8_symlink')) From e7aa6bd5863c29ebfb083210365bdadd49e8fea6 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Wed, 21 Jul 2021 02:03:19 +0200 Subject: [PATCH 29/32] [Refactor] First load all wallets, then back em --- src/wallet/init.cpp | 15 +++++++++------ test/functional/wallet_multiwallet.py | 5 ++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index c2ac22ec133e..5d88ba4124ae 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -219,19 +219,22 @@ bool InitLoadWallet() return false; } - // automatic backup + // add to wallets in use + vpwallets.emplace_back(pwallet); + } + + // automatic backup + // do this after loading all wallets, so unique fileids are checked properly + for (CWallet* pwallet: vpwallets) { std::string strWarning, strError; if (!AutoBackupWallet(*pwallet, strWarning, strError)) { if (!strWarning.empty()) { - UIWarning(strprintf("%s: %s", walletFile, strWarning)); + UIWarning(strprintf("%s: %s", pwallet->GetName(), strWarning)); } if (!strError.empty()) { - return UIError(strprintf("%s: %s", walletFile, strError)); + return UIError(strprintf("%s: %s", pwallet->GetName(), strError)); } } - - // add to wallets in use - vpwallets.emplace_back(pwallet); } return true; diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 6bb349a59c37..0f3803c08e76 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -74,10 +74,9 @@ def wallet_file(name): # should not initialize if there are duplicate wallets self.assert_start_raises_init_error(0, ['-wallet=w1', '-wallet=w1'], 'Error loading wallet w1. Duplicate -wallet filename specified.') - # !TODO: automatic backup changes the environment, fix in the next commit # should not initialize if one wallet is a copy of another - # shutil.copyfile(wallet_dir('w8'), wallet_dir('w8_copy')) - # self.assert_start_raises_init_error(0, ['-wallet=w8', '-wallet=w8_copy'], 'duplicates fileid') + shutil.copyfile(wallet_dir('w8'), wallet_dir('w8_copy')) + self.assert_start_raises_init_error(0, ['-wallet=w8', '-wallet=w8_copy'], 'duplicates fileid') # should not initialize if wallet file is a symlink os.symlink('w8', wallet_dir('w8_symlink')) From 524103f24a49ed3ad21cb3b422997a21d34ef769 Mon Sep 17 00:00:00 2001 From: furszy Date: Wed, 21 Jul 2021 12:02:43 -0300 Subject: [PATCH 30/32] Implement and connect CWallet::GetPathToFile to GUI. Co-authored-by: random-zebra --- src/qt/walletmodel.cpp | 2 +- src/wallet/db.h | 2 +- src/wallet/wallet.h | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 13fdbb5ecd0e..cee21970c4cb 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -122,7 +122,7 @@ bool WalletModel::upgradeWallet(std::string& upgradeError) QString WalletModel::getWalletPath() { - return QString::fromStdString(GetDataDir().string() + QDir::separator().toLatin1() + wallet->GetName()); + return QString::fromStdString(wallet->GetPathToDBFile().string()); } CAmount WalletModel::getBalance(const CCoinControl* coinControl, bool fIncludeDelegated, bool fUnlockedOnly, bool fIncludeShielded) const diff --git a/src/wallet/db.h b/src/wallet/db.h index 64445f4fd3ba..d553e9b84696 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -153,7 +153,7 @@ class CWalletDBWrapper unsigned int nLastSeen; unsigned int nLastFlushed; int64_t nLastWalletUpdate; - unsigned int GetUpdateCounter(); + fs::path GetPathToFile() { return env->Directory() / strFile; } private: /** BerkeleyDB specific */ diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 6293a8475b58..9ebc998ed09c 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -744,6 +744,9 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface */ const std::string& GetName() const { return m_name; } + /** Get the path to the wallet's db file */ + fs::path GetPathToDBFile() { return dbw->GetPathToFile(); } + /** Construct wallet with specified name and database implementation. */ CWallet(std::string name, std::unique_ptr dbw_in); ~CWallet(); From f76561155308fa594cb58b5df0026f3ee5bace7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Mon, 4 Jun 2018 23:15:03 +0100 Subject: [PATCH 31/32] bugfix: Remove dangling wallet env instance --- src/wallet/db.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 047df48fd4c7..ca6b066e7ee2 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -796,6 +796,7 @@ void CWalletDBWrapper::Flush(bool shutdown) { if (!IsDummy()) { env->Flush(shutdown); + if (shutdown) env = nullptr; } } From 056f4e8436e2e08c18c2c1fd71b7833e2e4007a5 Mon Sep 17 00:00:00 2001 From: furszy Date: Wed, 21 Jul 2021 12:30:59 -0300 Subject: [PATCH 32/32] [GUI] settings information widget setting the correct data directory. --- src/qt/pivx/settings/settingsinformationwidget.cpp | 11 +++-------- src/qt/pivx/settings/settingsinformationwidget.h | 1 - src/qt/rpcconsole.cpp | 3 --- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/qt/pivx/settings/settingsinformationwidget.cpp b/src/qt/pivx/settings/settingsinformationwidget.cpp index 90309f9bf2fb..a00bce96cda6 100644 --- a/src/qt/pivx/settings/settingsinformationwidget.cpp +++ b/src/qt/pivx/settings/settingsinformationwidget.cpp @@ -115,6 +115,7 @@ void SettingsInformationWidget::loadClientModel() ui->labelInfoAgent->setText(clientModel->clientName()); ui->labelInfoTime->setText(clientModel->formatClientStartupTime()); ui->labelInfoName->setText(QString::fromStdString(Params().NetworkIDString())); + ui->labelInfoDataDir->setText(clientModel->dataDir()); setNumConnections(clientModel->getNumConnections()); connect(clientModel, &ClientModel::numConnectionsChanged, this, &SettingsInformationWidget::setNumConnections); @@ -126,13 +127,6 @@ void SettingsInformationWidget::loadClientModel() } } -void SettingsInformationWidget::loadWalletModel() -{ - if (walletModel) { - ui->labelInfoDataDir->setText(walletModel->getWalletPath()); - } -} - void SettingsInformationWidget::setNumConnections(int count) { if (!clientModel) @@ -163,8 +157,9 @@ void SettingsInformationWidget::setMasternodeCount(const QString& strMasternodes void SettingsInformationWidget::openNetworkMonitor() { if (!rpcConsole) { - rpcConsole = new RPCConsole(0); + rpcConsole = new RPCConsole(nullptr); rpcConsole->setClientModel(clientModel); + rpcConsole->setWalletModel(walletModel); } rpcConsole->showNetwork(); } diff --git a/src/qt/pivx/settings/settingsinformationwidget.h b/src/qt/pivx/settings/settingsinformationwidget.h index 5737aaededb3..c29caf8aed13 100644 --- a/src/qt/pivx/settings/settingsinformationwidget.h +++ b/src/qt/pivx/settings/settingsinformationwidget.h @@ -22,7 +22,6 @@ class SettingsInformationWidget : public PWidget ~SettingsInformationWidget() override; void loadClientModel() override; - void loadWalletModel() override; void run(int type) override; void onError(QString error, int type) override; diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index dc8f99eccba3..c4768bde9b82 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -17,9 +17,6 @@ #include "chainparams.h" #include "netbase.h" #include "util/system.h" -#ifdef ENABLE_WALLET -#include "wallet/wallet.h" -#endif // ENABLE_WALLET #ifdef ENABLE_WALLET #include