From 6efd9644cfe31168db1841010cffa64dfe604e67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Tue, 17 Apr 2018 17:41:49 +0100 Subject: [PATCH 1/3] refactor: Drop CWalletRef typedef --- src/interfaces/node.cpp | 2 +- src/wallet/init.cpp | 8 ++++---- src/wallet/rpcwallet.cpp | 4 ++-- src/wallet/wallet.cpp | 2 +- src/wallet/wallet.h | 7 +++---- src/wallet/walletdb.cpp | 2 +- 6 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 919748f94..6d49117de 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -239,7 +239,7 @@ class NodeImpl : public Node { #ifdef ENABLE_WALLET std::vector> wallets; - for (CWalletRef wallet : ::vpwallets) { + for (CWallet* wallet : ::vpwallets) { wallets.emplace_back(MakeWallet(*wallet)); } return wallets; diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 2fd9aa1a6..860e1cfac 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -323,28 +323,28 @@ bool WalletInit::Open() const void WalletInit::Start(CScheduler& scheduler) const { - for (CWalletRef pwallet : vpwallets) { + for (CWallet* pwallet : vpwallets) { pwallet->postInitProcess(scheduler); } } void WalletInit::Flush() const { - for (CWalletRef pwallet : vpwallets) { + for (CWallet* pwallet : vpwallets) { pwallet->Flush(false); } } void WalletInit::Stop() const { - for (CWalletRef pwallet : vpwallets) { + for (CWallet* pwallet : vpwallets) { pwallet->Flush(true); } } void WalletInit::Close() const { - for (CWalletRef pwallet : vpwallets) { + for (CWallet* pwallet : vpwallets) { delete pwallet; } vpwallets.clear(); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 56bdc0695..5298283b4 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -46,7 +46,7 @@ CWallet *GetWalletForJSONRPCRequest(const JSONRPCRequest& request) if (request.URI.substr(0, WALLET_ENDPOINT_BASE.size()) == WALLET_ENDPOINT_BASE) { // wallet endpoint was used std::string requestedWallet = urlDecode(request.URI.substr(WALLET_ENDPOINT_BASE.size())); - for (CWalletRef pwallet : ::vpwallets) { + for (CWallet* pwallet : ::vpwallets) { if (pwallet->GetName() == requestedWallet) { return pwallet; } @@ -2862,7 +2862,7 @@ UniValue listwallets(const JSONRPCRequest& request) UniValue obj(UniValue::VARR); - for (CWalletRef pwallet : vpwallets) { + for (CWallet* pwallet : vpwallets) { if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 45c85a791..dcf566c1e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -33,7 +33,7 @@ #include -std::vector vpwallets; +std::vector vpwallets; /** Transaction fee set by the user */ CFeeRate payTxFee(DEFAULT_TRANSACTION_FEE); unsigned int nTxConfirmTarget = DEFAULT_TX_CONFIRM_TARGET; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index b85f374a0..ceac4ac58 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -32,8 +32,7 @@ #include #include -typedef CWallet* CWalletRef; -extern std::vector vpwallets; +extern std::vector vpwallets; /** * Settings @@ -1230,10 +1229,10 @@ std::vector GetAllDestinationsForKey(const CPubKey& key); class WalletRescanReserver { private: - CWalletRef m_wallet; + CWallet* m_wallet; bool m_could_reserve; public: - explicit WalletRescanReserver(CWalletRef w) : m_wallet(w), m_could_reserve(false) {} + explicit WalletRescanReserver(CWallet* w) : m_wallet(w), m_could_reserve(false) {} bool reserve() { diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index bcc7cf877..0e8a12ee9 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -756,7 +756,7 @@ void MaybeCompactWalletDB() return; } - for (CWalletRef pwallet : vpwallets) { + for (CWallet* pwallet : vpwallets) { WalletDatabase& dbh = pwallet->GetDBHandle(); unsigned int nUpdateCounter = dbh.nUpdateCounter; From 373aee26c3df233f4e0a7e806f45ac7cb5aab1e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Tue, 17 Apr 2018 18:22:23 +0100 Subject: [PATCH 2/3] wallet: Add AddWallet, RemoveWallet, GetWallet and GetWallets With these new functions all vpwallets usage are removed and vpwallets is now a static variable (no external linkage). --- src/interfaces/node.cpp | 2 +- src/qt/test/wallettests.cpp | 6 +++--- src/rpc/misc.cpp | 2 +- src/wallet/init.cpp | 12 +++++------ src/wallet/rpcwallet.cpp | 18 ++++++++-------- src/wallet/test/wallet_tests.cpp | 12 +++++------ src/wallet/wallet.cpp | 35 +++++++++++++++++++++++++++++++- src/wallet/wallet.h | 21 +++++++++++-------- src/wallet/walletdb.cpp | 2 +- 9 files changed, 72 insertions(+), 38 deletions(-) diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 6d49117de..c8ffca02c 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -239,7 +239,7 @@ class NodeImpl : public Node { #ifdef ENABLE_WALLET std::vector> wallets; - for (CWallet* wallet : ::vpwallets) { + for (CWallet* wallet : GetWallets()) { wallets.emplace_back(MakeWallet(*wallet)); } return wallets; diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index dcc834c35..56d2d3819 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -180,9 +180,9 @@ void TestGUI() TransactionView transactionView(platformStyle.get()); auto node = interfaces::MakeNode(); OptionsModel optionsModel(*node); - vpwallets.insert(vpwallets.begin(), &wallet); - WalletModel walletModel(std::move(node->getWallets()[0]), *node, platformStyle.get(), &optionsModel); - vpwallets.erase(vpwallets.begin()); + AddWallet(&wallet); + WalletModel walletModel(std::move(node->getWallets().back()), *node, platformStyle.get(), &optionsModel); + RemoveWallet(&wallet); sendCoinsDialog.setModel(&walletModel); transactionView.setModel(&walletModel); diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 49e865a64..ba3ea7055 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -69,7 +69,7 @@ UniValue validateaddress(const JSONRPCRequest& request) { #ifdef ENABLE_WALLET - if (!::vpwallets.empty() && IsDeprecatedRPCEnabled("validateaddress")) { + if (!GetWallets().empty() && IsDeprecatedRPCEnabled("validateaddress")) { ret.pushKVs(getaddressinfo(request)); } #endif diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 860e1cfac..8b834e462 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -315,7 +315,7 @@ bool WalletInit::Open() const if (!pwallet) { return false; } - vpwallets.push_back(pwallet); + AddWallet(pwallet); } return true; @@ -323,29 +323,29 @@ bool WalletInit::Open() const void WalletInit::Start(CScheduler& scheduler) const { - for (CWallet* pwallet : vpwallets) { + for (CWallet* pwallet : GetWallets()) { pwallet->postInitProcess(scheduler); } } void WalletInit::Flush() const { - for (CWallet* pwallet : vpwallets) { + for (CWallet* pwallet : GetWallets()) { pwallet->Flush(false); } } void WalletInit::Stop() const { - for (CWallet* pwallet : vpwallets) { + for (CWallet* pwallet : GetWallets()) { pwallet->Flush(true); } } void WalletInit::Close() const { - for (CWallet* pwallet : vpwallets) { + for (CWallet* pwallet : GetWallets()) { + RemoveWallet(pwallet); delete pwallet; } - vpwallets.clear(); } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 5298283b4..9875a2697 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -46,14 +46,13 @@ CWallet *GetWalletForJSONRPCRequest(const JSONRPCRequest& request) if (request.URI.substr(0, WALLET_ENDPOINT_BASE.size()) == WALLET_ENDPOINT_BASE) { // wallet endpoint was used std::string requestedWallet = urlDecode(request.URI.substr(WALLET_ENDPOINT_BASE.size())); - for (CWallet* pwallet : ::vpwallets) { - if (pwallet->GetName() == requestedWallet) { - return pwallet; - } - } - throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Requested wallet does not exist or is not loaded"); + CWallet* pwallet = GetWallet(requestedWallet); + if (!pwallet) throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Requested wallet does not exist or is not loaded"); + return pwallet; } - return ::vpwallets.size() == 1 || (request.fHelp && ::vpwallets.size() > 0) ? ::vpwallets[0] : nullptr; + + std::vector wallets = GetWallets(); + return wallets.size() == 1 || (request.fHelp && wallets.size() > 0) ? wallets[0] : nullptr; } std::string HelpRequiringPassphrase(CWallet * const pwallet) @@ -67,7 +66,7 @@ bool EnsureWalletIsAvailable(CWallet * const pwallet, bool avoidException) { if (pwallet) return true; if (avoidException) return false; - if (::vpwallets.empty()) { + if (GetWallets().empty()) { // Note: It isn't currently possible to trigger this error because // wallet RPC methods aren't registered unless a wallet is loaded. But // this error is being kept as a precaution, because it's possible in @@ -2862,8 +2861,7 @@ UniValue listwallets(const JSONRPCRequest& request) UniValue obj(UniValue::VARR); - for (CWallet* pwallet : vpwallets) { - + for (CWallet* pwallet : GetWallets()) { if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 57705926a..99c963a34 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -74,7 +74,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) // after. { CWallet wallet("dummy", WalletDatabase::CreateDummy()); - vpwallets.insert(vpwallets.begin(), &wallet); + AddWallet(&wallet); UniValue keys; keys.setArray(); UniValue key; @@ -105,7 +105,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) "downloading and rescanning the relevant blocks (see -reindex and -rescan " "options).\"}},{\"success\":true}]", 0, oldTip->GetBlockTimeMax(), TIMESTAMP_WINDOW)); - vpwallets.erase(vpwallets.begin()); + RemoveWallet(&wallet); } } @@ -140,9 +140,9 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) JSONRPCRequest request; request.params.setArray(); request.params.push_back((pathTemp / "wallet.backup").string()); - vpwallets.insert(vpwallets.begin(), &wallet); + AddWallet(&wallet); ::dumpwallet(request); - vpwallets.erase(vpwallets.begin()); + RemoveWallet(&wallet); } // Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME @@ -153,9 +153,9 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) JSONRPCRequest request; request.params.setArray(); request.params.push_back((pathTemp / "wallet.backup").string()); - vpwallets.insert(vpwallets.begin(), &wallet); + AddWallet(&wallet); ::importwallet(request); - vpwallets.erase(vpwallets.begin()); + RemoveWallet(&wallet); LOCK(wallet.cs_wallet); BOOST_CHECK_EQUAL(wallet.mapWallet.size(), 3U); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index dcf566c1e..fcc4cfb1f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -28,12 +28,45 @@ #include #include +#include #include #include #include -std::vector vpwallets; +static std::vector vpwallets; + +bool AddWallet(CWallet* wallet) +{ + assert(wallet); + std::vector::const_iterator i = std::find(vpwallets.begin(), vpwallets.end(), wallet); + if (i != vpwallets.end()) return false; + vpwallets.push_back(wallet); + return true; +} + +bool RemoveWallet(CWallet* wallet) +{ + assert(wallet); + std::vector::iterator i = std::find(vpwallets.begin(), vpwallets.end(), wallet); + if (i == vpwallets.end()) return false; + vpwallets.erase(i); + return true; +} + +std::vector GetWallets() +{ + return vpwallets; +} + +CWallet* GetWallet(const std::string& name) +{ + for (CWallet* wallet : vpwallets) { + if (wallet->GetName() == name) return wallet; + } + return nullptr; +} + /** Transaction fee set by the user */ CFeeRate payTxFee(DEFAULT_TRANSACTION_FEE); unsigned int nTxConfirmTarget = DEFAULT_TX_CONFIRM_TARGET; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index ceac4ac58..67f0b3adb 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -32,7 +32,10 @@ #include #include -extern std::vector vpwallets; +bool AddWallet(CWallet* wallet); +bool RemoveWallet(CWallet* wallet); +std::vector GetWallets(); +CWallet* GetWallet(const std::string& name); /** * Settings @@ -267,7 +270,7 @@ public: //Get the marginal bytes of spending the specified output int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet); -/** +/** * A transaction with a bunch of additional info that only the owner cares about. * It includes any unrecorded transactions needed to link it back to the block chain. */ @@ -652,7 +655,7 @@ struct CoinEligibilityFilter }; class WalletRescanReserver; //forward declarations for ScanForWalletTransactions/RescanFromTime -/** +/** * A CWallet is an extension of a keystore, which also maintains a set of transactions and balances, * and provides the ability to create new transactions. */ @@ -902,7 +905,7 @@ public: void GetKeyBirthTimes(std::map &mapKeyBirth) const; unsigned int ComputeTimeSmart(const CWalletTx& wtx) const; - /** + /** * Increment the next transaction order id * @return next transaction order id */ @@ -1031,7 +1034,7 @@ public: } void GetScriptForMining(std::shared_ptr &script); - + unsigned int GetKeyPoolSize() { AssertLockHeld(cs_wallet); // set{Ex,In}ternalKeyPool @@ -1056,7 +1059,7 @@ public: //! Flush wallet (bitdb flush) void Flush(bool shutdown=false); - /** + /** * Address book entry changed. * @note called with lock cs_wallet held. */ @@ -1065,7 +1068,7 @@ public: const std::string &purpose, ChangeType status)> NotifyAddressBookChanged; - /** + /** * Wallet transaction added, removed or updated. * @note called with lock cs_wallet held. */ @@ -1112,7 +1115,7 @@ public: /* Generates a new HD master key (will not be activated) */ CPubKey GenerateNewHDMasterKey(); - + /* Set the current HD master key (will reset the chain child index counters) Sets the master key's version based on the current wallet version (so the caller must ensure the current wallet version is correct before calling @@ -1183,7 +1186,7 @@ public: }; -/** +/** * DEPRECATED Account information. * Stored in wallet with key "acc"+string account name. */ diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 0e8a12ee9..5b275131a 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -756,7 +756,7 @@ void MaybeCompactWalletDB() return; } - for (CWallet* pwallet : vpwallets) { + for (CWallet* pwallet : GetWallets()) { WalletDatabase& dbh = pwallet->GetDBHandle(); unsigned int nUpdateCounter = dbh.nUpdateCounter; From 3c058fdcc8a71d17296973cb7f09e44a310df22e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Wed, 18 Apr 2018 13:46:11 +0100 Subject: [PATCH 3/3] wallet: Add HasWallets --- src/rpc/misc.cpp | 2 +- src/wallet/rpcwallet.cpp | 2 +- src/wallet/wallet.cpp | 5 +++++ src/wallet/wallet.h | 1 + 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index ba3ea7055..6754407db 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -69,7 +69,7 @@ UniValue validateaddress(const JSONRPCRequest& request) { #ifdef ENABLE_WALLET - if (!GetWallets().empty() && IsDeprecatedRPCEnabled("validateaddress")) { + if (HasWallets() && IsDeprecatedRPCEnabled("validateaddress")) { ret.pushKVs(getaddressinfo(request)); } #endif diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 9875a2697..9b5fb0b06 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -66,7 +66,7 @@ bool EnsureWalletIsAvailable(CWallet * const pwallet, bool avoidException) { if (pwallet) return true; if (avoidException) return false; - if (GetWallets().empty()) { + if (!HasWallets()) { // Note: It isn't currently possible to trigger this error because // wallet RPC methods aren't registered unless a wallet is loaded. But // this error is being kept as a precaution, because it's possible in diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index fcc4cfb1f..8c392434f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -54,6 +54,11 @@ bool RemoveWallet(CWallet* wallet) return true; } +bool HasWallets() +{ + return !vpwallets.empty(); +} + std::vector GetWallets() { return vpwallets; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 67f0b3adb..dd165de82 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -34,6 +34,7 @@ bool AddWallet(CWallet* wallet); bool RemoveWallet(CWallet* wallet); +bool HasWallets(); std::vector GetWallets(); CWallet* GetWallet(const std::string& name);