From c376b64a4f6249db3fb134e0e2362ad031840b23 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 24 May 2018 11:29:07 +0200 Subject: [PATCH] Merge #13063: Use shared pointer to retain wallet instance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 80b4910f7d87983f50047074c3c2397b0a5c4e92 wallet: Use shared pointer to retain wallet instance (João Barbosa) Pull request description: Currently there are 3 places where it makes sense to retain a wallet shared pointer: - `vpwallets`; - `interfaces::Wallet` interface instance - used by the UI; - wallet RPC functions - given by `GetWalletForJSONRPCRequest`. The way it is now it is possible to have, for instance, listunspent RPC and in parallel unload the wallet (once #13111 is merged) without blocking. Once the RPC finishes, the shared pointer will release the wallet. It is also possible to get all existing wallets without blocking because the caller keeps a local list of shared pointers. This is mostly relevant for wallet unloading. This PR replaces #11402. Tree-SHA512: b7e37c7e1ab56626085afe2d40b1628e8d4f0dbda08df01b7e618ecd2d894ce9b83d4219443f444ba889096286eff002f163cb0a48f37063b62e9ba4ccfa6cce --- src/coinjoin/coinjoin-client.cpp | 8 +- src/coinjoin/coinjoin-util.cpp | 10 +- src/coinjoin/coinjoin-util.h | 6 +- src/interfaces/node.cpp | 6 +- src/interfaces/wallet.cpp | 5 +- src/interfaces/wallet.h | 2 +- src/qt/test/wallettests.cpp | 24 ++-- src/rpc/coinjoin.cpp | 6 +- src/rpc/governance.cpp | 12 +- src/rpc/masternode.cpp | 3 +- src/rpc/rawtransaction.cpp | 3 +- src/rpc/rpcevo.cpp | 21 ++- src/ui_interface.h | 3 +- src/wallet/init.cpp | 14 +- src/wallet/rpcdump.cpp | 36 +++-- src/wallet/rpcwallet.cpp | 205 +++++++++++++++++++++-------- src/wallet/rpcwallet.h | 2 +- src/wallet/test/coinjoin_tests.cpp | 10 +- src/wallet/test/wallet_tests.cpp | 36 ++--- src/wallet/wallet.cpp | 31 ++--- src/wallet/wallet.h | 11 +- src/wallet/walletdb.cpp | 2 +- 22 files changed, 286 insertions(+), 170 deletions(-) diff --git a/src/coinjoin/coinjoin-client.cpp b/src/coinjoin/coinjoin-client.cpp index 1e7654c57ddb..acf3524013fc 100644 --- a/src/coinjoin/coinjoin-client.cpp +++ b/src/coinjoin/coinjoin-client.cpp @@ -1345,12 +1345,12 @@ bool CCoinJoinClientSession::PrepareDenominate(int nMinRounds, int nMaxRounds, s ++nSteps; continue; } - CWallet* pwallet = GetWallet(mixingWallet.GetName()); + const auto pwallet = GetWallet(mixingWallet.GetName()); if (!pwallet) { strErrorRet ="Couldn't get wallet pointer"; return false; } - scriptDenom = keyHolderStorage.AddKey(pwallet); + scriptDenom = keyHolderStorage.AddKey(pwallet.get()); } vecPSInOutPairsRet.emplace_back(entry, CTxOut(nDenomAmount, scriptDenom)); // step is complete @@ -1434,7 +1434,7 @@ bool CCoinJoinClientSession::MakeCollateralAmounts(const CompactTallyItem& tally return false; } - CWallet* pwallet = GetWallet(mixingWallet.GetName()); + const auto pwallet = GetWallet(mixingWallet.GetName()); if (!pwallet) { LogPrint(BCLog::COINJOIN, "CCoinJoinClientSession::%s -- Couldn't get wallet pointer\n", __func__); @@ -1607,7 +1607,7 @@ bool CCoinJoinClientSession::CreateDenominated(CAmount nBalanceToDenominate, con return false; } - CWallet* pwallet = GetWallet(mixingWallet.GetName()); + const auto pwallet = GetWallet(mixingWallet.GetName()); if (!pwallet) { LogPrint(BCLog::COINJOIN, "CCoinJoinClientSession::%s -- Couldn't get wallet pointer\n", __func__); diff --git a/src/coinjoin/coinjoin-util.cpp b/src/coinjoin/coinjoin-util.cpp index bc13aa1728c3..bd11552d055d 100644 --- a/src/coinjoin/coinjoin-util.cpp +++ b/src/coinjoin/coinjoin-util.cpp @@ -84,9 +84,9 @@ void CKeyHolderStorage::ReturnAll() } } -CTransactionBuilderOutput::CTransactionBuilderOutput(CTransactionBuilder* pTxBuilderIn, CWallet* pwalletIn, CAmount nAmountIn) : +CTransactionBuilderOutput::CTransactionBuilderOutput(CTransactionBuilder* pTxBuilderIn, std::shared_ptr pwalletIn, CAmount nAmountIn) : pTxBuilder(pTxBuilderIn), - key(pwalletIn), + key(pwalletIn.get()), nAmount(nAmountIn) { assert(pTxBuilder); @@ -105,9 +105,9 @@ bool CTransactionBuilderOutput::UpdateAmount(const CAmount nNewAmount) return true; } -CTransactionBuilder::CTransactionBuilder(CWallet* pwalletIn, const CompactTallyItem& tallyItemIn) : +CTransactionBuilder::CTransactionBuilder(std::shared_ptr pwalletIn, const CompactTallyItem& tallyItemIn) : pwallet(pwalletIn), - dummyReserveKey(pwalletIn), + dummyReserveKey(pwalletIn.get()), tallyItem(tallyItemIn) { // Generate a feerate which will be used to consider if the remainder is dust and will go into fees or not @@ -139,7 +139,7 @@ CTransactionBuilder::CTransactionBuilder(CWallet* pwalletIn, const CompactTallyI for (const auto& coin : tallyItem.vecInputCoins) { const CScript& scriptPubKey = coin.txout.scriptPubKey; SignatureData sigdata; - bool res = ProduceSignature(DummySignatureCreator(pwallet), scriptPubKey, sigdata); + bool res = ProduceSignature(DummySignatureCreator(pwallet.get()), scriptPubKey, sigdata); assert(res); UpdateTransaction(dummyTx, nIn, sigdata); nIn++; diff --git a/src/coinjoin/coinjoin-util.h b/src/coinjoin/coinjoin-util.h index c07882c31f53..dc261e39236a 100644 --- a/src/coinjoin/coinjoin-util.h +++ b/src/coinjoin/coinjoin-util.h @@ -53,7 +53,7 @@ class CTransactionBuilderOutput CScript script; public: - CTransactionBuilderOutput(CTransactionBuilder* pTxBuilderIn, CWallet* pwalletIn, CAmount nAmountIn); + CTransactionBuilderOutput(CTransactionBuilder* pTxBuilderIn, std::shared_ptr pwalletIn, CAmount nAmountIn); CTransactionBuilderOutput(CTransactionBuilderOutput&&) = delete; CTransactionBuilderOutput& operator=(CTransactionBuilderOutput&&) = delete; /// Get the scriptPubKey of this output @@ -76,7 +76,7 @@ class CTransactionBuilderOutput class CTransactionBuilder { /// Wallet the transaction will be build for - CWallet* pwallet{nullptr}; + std::shared_ptr pwallet; /// See CTransactionBuilder() for initialization CCoinControl coinControl; /// Dummy since we anyway use tallyItem's destination as change destination in coincontrol. @@ -99,7 +99,7 @@ class CTransactionBuilder friend class CTransactionBuilderOutput; public: - CTransactionBuilder(CWallet* pwalletIn, const CompactTallyItem& tallyItemIn); + CTransactionBuilder(std::shared_ptr pwalletIn, const CompactTallyItem& tallyItemIn); ~CTransactionBuilder(); /// Check it would be possible to add a single output with the amount nAmount. Returns true if its possible and false if not. bool CouldAddOutput(CAmount nAmountOutput) const; diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 72006d0391a4..bb05f4ac6f48 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -356,8 +356,8 @@ class NodeImpl : public Node { #ifdef ENABLE_WALLET std::vector> wallets; - for (CWalletRef wallet : GetWallets()) { - wallets.emplace_back(MakeWallet(*wallet)); + for (const std::shared_ptr& wallet : GetWallets()) { + wallets.emplace_back(MakeWallet(wallet)); } return wallets; #else @@ -390,7 +390,7 @@ class NodeImpl : public Node std::unique_ptr handleLoadWallet(LoadWalletFn fn) override { CHECK_WALLET( - return MakeHandler(::uiInterface.LoadWallet.connect([fn](CWallet* wallet) { fn(MakeWallet(*wallet)); }))); + return MakeHandler(::uiInterface.LoadWallet.connect([fn](std::shared_ptr wallet) { fn(MakeWallet(wallet)); }))); } std::unique_ptr handleNotifyNumConnectionsChanged(NotifyNumConnectionsChangedFn fn) override { diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index ac9da28f2294..a1b696e32b26 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -175,7 +175,7 @@ class WalletImpl : public Wallet public: CoinJoinImpl m_coinjoin; - WalletImpl(CWallet& wallet) : m_wallet(wallet), m_coinjoin(wallet) {} + WalletImpl(const std::shared_ptr& wallet) : m_shared_wallet(wallet), m_wallet(*wallet.get()), m_coinjoin(*wallet.get()) {} void markDirty() override { @@ -518,11 +518,12 @@ class WalletImpl : public Wallet return MakeHandler(m_wallet.NotifyWatchonlyChanged.connect(fn)); } + std::shared_ptr m_shared_wallet; CWallet& m_wallet; }; } // namespace -std::unique_ptr MakeWallet(CWallet& wallet) { return MakeUnique(wallet); } +std::unique_ptr MakeWallet(const std::shared_ptr& wallet) { return MakeUnique(wallet); } } // namespace interfaces diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index efea81912a38..9113e13ee480 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -379,7 +379,7 @@ struct WalletTxOut //! Return implementation of Wallet interface. This function will be undefined //! in builds where ENABLE_WALLET is false. -std::unique_ptr MakeWallet(CWallet& wallet); +std::unique_ptr MakeWallet(const std::shared_ptr& wallet); } // namespace interfaces diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 16f9ffbbe8e7..4380bccf6490 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -119,22 +119,22 @@ void TestGUI() for (int i = 0; i < 5; ++i) { test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey())); } - CWallet wallet("mock", WalletDatabase::CreateMock()); - AddWallet(&wallet); + std::shared_ptr wallet = std::make_shared("mock", WalletDatabase::CreateMock()); + AddWallet(wallet); bool firstRun; - wallet.LoadWallet(firstRun); + wallet->LoadWallet(firstRun); { - LOCK(wallet.cs_wallet); - wallet.SetAddressBook(test.coinbaseKey.GetPubKey().GetID(), "", "receive"); - wallet.AddKeyPubKey(test.coinbaseKey, test.coinbaseKey.GetPubKey()); + LOCK(wallet->cs_wallet); + wallet->SetAddressBook(test.coinbaseKey.GetPubKey().GetID(), "", "receive"); + wallet->AddKeyPubKey(test.coinbaseKey, test.coinbaseKey.GetPubKey()); } { LOCK(cs_main); - WalletRescanReserver reserver(&wallet); + WalletRescanReserver reserver(wallet.get()); reserver.reserve(); - wallet.ScanForWalletTransactions(chainActive.Genesis(), nullptr, reserver, true); + wallet->ScanForWalletTransactions(chainActive.Genesis(), nullptr, reserver, true); } - wallet.SetBroadcastTransactions(true); + wallet->SetBroadcastTransactions(true); // Create widgets for sending coins and listing transactions. SendCoinsDialog sendCoinsDialog; @@ -149,8 +149,8 @@ void TestGUI() // Send two transactions, and verify they are added to transaction list. TransactionTableModel* transactionTableModel = walletModel.getTransactionTableModel(); QCOMPARE(transactionTableModel->rowCount({}), 105); - uint256 txid1 = SendCoins(wallet, sendCoinsDialog, CKeyID(), 5 * COIN); - uint256 txid2 = SendCoins(wallet, sendCoinsDialog, CKeyID(), 10 * COIN); + uint256 txid1 = SendCoins(*wallet.get(), sendCoinsDialog, CKeyID(), 5 * COIN); + uint256 txid2 = SendCoins(*wallet.get(), sendCoinsDialog, CKeyID(), 10 * COIN); QCOMPARE(transactionTableModel->rowCount({}), 107); QVERIFY(FindTx(*transactionTableModel, txid1).isValid()); QVERIFY(FindTx(*transactionTableModel, txid2).isValid()); @@ -217,7 +217,7 @@ void TestGUI() QPushButton* removeRequestButton = receiveCoinsDialog.findChild("removeRequestButton"); removeRequestButton->click(); QCOMPARE(requestTableModel->rowCount({}), currentRowCount-1); - RemoveWallet(&wallet); + RemoveWallet(wallet); } } diff --git a/src/rpc/coinjoin.cpp b/src/rpc/coinjoin.cpp index e7d0f1270da8..ca6e95a7e9e0 100644 --- a/src/rpc/coinjoin.cpp +++ b/src/rpc/coinjoin.cpp @@ -16,7 +16,8 @@ #ifdef ENABLE_WALLET UniValue coinjoin(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) return NullUniValue; @@ -143,7 +144,8 @@ UniValue getcoinjoininfo(const JSONRPCRequest& request) obj.pushKV("queue_size", coinJoinClientQueueManager.GetQueueSize()); - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!pwallet) { return obj; } diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index 1b58840faf7d..677de24d4210 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -137,7 +137,8 @@ void gobject_prepare_help(CWallet* const pwallet) UniValue gobject_prepare(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (request.fHelp || (request.params.size() != 5 && request.params.size() != 6 && request.params.size() != 8)) gobject_prepare_help(pwallet); @@ -244,7 +245,8 @@ void gobject_list_prepared_help(CWallet* const pwallet) UniValue gobject_list_prepared(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (request.fHelp || (request.params.size() > 2)) { gobject_list_prepared_help(pwallet); } @@ -570,7 +572,8 @@ void gobject_vote_many_help(CWallet* const pwallet) UniValue gobject_vote_many(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (request.fHelp || request.params.size() != 4) gobject_vote_many_help(pwallet); @@ -624,7 +627,8 @@ void gobject_vote_alias_help(CWallet* const pwallet) UniValue gobject_vote_alias(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (request.fHelp || request.params.size() != 5) gobject_vote_alias_help(pwallet); diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index b98fd55e08fe..e74c43cf835b 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -200,7 +200,8 @@ void masternode_outputs_help() UniValue masternode_outputs(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (request.fHelp) masternode_outputs_help(); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 842a135b6dfd..8d2ab8c97f5f 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -966,7 +966,8 @@ UniValue signrawtransactionwithkey(const JSONRPCRequest& request) UniValue signrawtransaction(const JSONRPCRequest& request) { #ifdef ENABLE_WALLET - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); #endif if (request.fHelp || request.params.size() < 1 || request.params.size() > 4) diff --git a/src/rpc/rpcevo.cpp b/src/rpc/rpcevo.cpp index d78b55bbfefb..4cdb9f969cf6 100644 --- a/src/rpc/rpcevo.cpp +++ b/src/rpc/rpcevo.cpp @@ -403,7 +403,8 @@ void protx_register_submit_help(CWallet* const pwallet) // handles register, register_prepare and register_fund in one method UniValue protx_register(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); bool isExternalRegister = request.params[0].get_str() == "register"; bool isFundRegister = request.params[0].get_str() == "register_fund"; bool isPrepareRegister = request.params[0].get_str() == "register_prepare"; @@ -564,7 +565,8 @@ UniValue protx_register(const JSONRPCRequest& request) UniValue protx_register_submit(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (request.fHelp || request.params.size() != 3) { protx_register_submit_help(pwallet); } @@ -618,7 +620,8 @@ void protx_update_service_help(CWallet* const pwallet) UniValue protx_update_service(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (request.fHelp || (request.params.size() < 4 || request.params.size() > 6)) protx_update_service_help(pwallet); @@ -713,7 +716,8 @@ void protx_update_registrar_help(CWallet* const pwallet) UniValue protx_update_registrar(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (request.fHelp || (request.params.size() != 5 && request.params.size() != 6)) { protx_update_registrar_help(pwallet); } @@ -801,7 +805,8 @@ void protx_revoke_help(CWallet* const pwallet) UniValue protx_revoke(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (request.fHelp || (request.params.size() < 3 || request.params.size() > 5)) { protx_revoke_help(pwallet); } @@ -962,7 +967,8 @@ UniValue protx_list(const JSONRPCRequest& request) } #ifdef ENABLE_WALLET - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); #else CWallet* const pwallet = nullptr; #endif @@ -1060,7 +1066,8 @@ UniValue protx_info(const JSONRPCRequest& request) } #ifdef ENABLE_WALLET - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); #else CWallet* const pwallet = nullptr; #endif diff --git a/src/ui_interface.h b/src/ui_interface.h index 3a51ad4eca9c..21be5e9b1f7b 100644 --- a/src/ui_interface.h +++ b/src/ui_interface.h @@ -6,6 +6,7 @@ #ifndef BITCOIN_UI_INTERFACE_H #define BITCOIN_UI_INTERFACE_H +#include #include #include @@ -93,7 +94,7 @@ class CClientUIInterface boost::signals2::signal NotifyAlertChanged; /** A wallet has been loaded. */ - boost::signals2::signal LoadWallet; + boost::signals2::signal wallet)> LoadWallet; /** * Show progress e.g. for verifychain. diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index eeaed778c10d..aa0b4d183541 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -400,7 +400,8 @@ bool WalletInit::Open() const } for (const std::string& walletFile : gArgs.GetArgs("-wallet")) { - if (!CWallet::CreateWalletFromFile(walletFile, fs::absolute(walletFile, GetWalletDir()))) { + std::shared_ptr pwallet = CWallet::CreateWalletFromFile(walletFile, fs::absolute(walletFile, GetWalletDir())); + if (!pwallet) { return false; } } @@ -410,7 +411,7 @@ bool WalletInit::Open() const void WalletInit::Start(CScheduler& scheduler) const { - for (CWallet* pwallet : GetWallets()) { + for (const std::shared_ptr& pwallet : GetWallets()) { pwallet->postInitProcess(); } @@ -424,7 +425,7 @@ void WalletInit::Start(CScheduler& scheduler) const void WalletInit::Flush() const { - for (CWallet* pwallet : GetWallets()) { + for (const std::shared_ptr& pwallet : GetWallets()) { if (CCoinJoinClientOptions::IsEnabled()) { // Stop CoinJoin, release keys auto it = coinJoinClientManagers.find(pwallet->GetName()); @@ -437,23 +438,22 @@ void WalletInit::Flush() const void WalletInit::Stop() const { - for (CWallet* pwallet : GetWallets()) { + for (const std::shared_ptr& pwallet : GetWallets()) { pwallet->Flush(true); } } void WalletInit::Close() const { - for (CWallet* pwallet : GetWallets()) { + for (const std::shared_ptr& pwallet : GetWallets()) { RemoveWallet(pwallet); - delete pwallet; } } void WalletInit::AutoLockMasternodeCollaterals() const { // we can't do this before DIP3 is fully initialized - for (CWallet* pwallet : GetWallets()) { + for (const auto pwallet : GetWallets()) { pwallet->AutoLockMasternodeCollaterals(); } } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index bec7a206d161..77730007cc92 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -68,7 +68,8 @@ std::string DecodeDumpString(const std::string &str) { UniValue importprivkey(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -159,7 +160,8 @@ UniValue importprivkey(const JSONRPCRequest& request) UniValue abortrescan(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -219,7 +221,8 @@ void ImportAddress(CWallet * const pwallet, const CTxDestination& dest, const st UniValue importaddress(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -303,7 +306,8 @@ UniValue importaddress(const JSONRPCRequest& request) UniValue importprunedfunds(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -365,7 +369,8 @@ UniValue importprunedfunds(const JSONRPCRequest& request) UniValue removeprunedfunds(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -403,7 +408,8 @@ UniValue removeprunedfunds(const JSONRPCRequest& request) UniValue importpubkey(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -476,7 +482,8 @@ UniValue importpubkey(const JSONRPCRequest& request) UniValue importwallet(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -609,7 +616,8 @@ UniValue importwallet(const JSONRPCRequest& request) UniValue importelectrumwallet(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) return NullUniValue; @@ -747,7 +755,8 @@ UniValue importelectrumwallet(const JSONRPCRequest& request) UniValue dumpprivkey(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -789,7 +798,8 @@ UniValue dumpprivkey(const JSONRPCRequest& request) UniValue dumphdinfo(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) return NullUniValue; @@ -833,7 +843,8 @@ UniValue dumphdinfo(const JSONRPCRequest& request) UniValue dumpwallet(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -1310,7 +1321,8 @@ int64_t GetImportTimestamp(const UniValue& data, int64_t now) UniValue importmulti(const JSONRPCRequest& mainRequest) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(mainRequest); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(mainRequest); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, mainRequest.fHelp)) { return NullUniValue; } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 49ebff93a116..ee0474c91796 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -43,17 +43,17 @@ static const std::string WALLET_ENDPOINT_BASE = "/wallet/"; -CWallet *GetWalletForJSONRPCRequest(const JSONRPCRequest& request) +std::shared_ptr 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())); - CWallet* pwallet = GetWallet(requestedWallet); + std::shared_ptr pwallet = GetWallet(requestedWallet); if (!pwallet) throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Requested wallet does not exist or is not loaded"); return pwallet; } - std::vector wallets = GetWallets(); + std::vector> wallets = GetWallets(); return wallets.size() == 1 || (request.fHelp && wallets.size() > 0) ? wallets[0] : nullptr; } @@ -134,7 +134,9 @@ std::string LabelFromValue(const UniValue& value) UniValue getnewaddress(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -189,7 +191,9 @@ CTxDestination GetLabelDestination(CWallet* const pwallet, const std::string& la UniValue getaccountaddress(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -230,7 +234,9 @@ UniValue getaccountaddress(const JSONRPCRequest& request) UniValue getrawchangeaddress(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -268,7 +274,9 @@ UniValue getrawchangeaddress(const JSONRPCRequest& request) UniValue setlabel(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -332,7 +340,9 @@ UniValue setlabel(const JSONRPCRequest& request) UniValue getaccount(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -375,7 +385,9 @@ UniValue getaccount(const JSONRPCRequest& request) UniValue getaddressesbyaccount(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -465,7 +477,9 @@ static CTransactionRef SendMoney(CWallet * const pwallet, const CTxDestination & UniValue sendtoaddress(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -564,7 +578,9 @@ UniValue instantsendtoaddress(const JSONRPCRequest& request) UniValue listaddressgroupings(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -621,7 +637,8 @@ UniValue listaddressgroupings(const JSONRPCRequest& request) UniValue listaddressbalances(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) return NullUniValue; @@ -663,7 +680,9 @@ UniValue listaddressbalances(const JSONRPCRequest& request) UniValue signmessage(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -724,7 +743,9 @@ UniValue signmessage(const JSONRPCRequest& request) UniValue getreceivedbyaddress(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -791,7 +812,9 @@ UniValue getreceivedbyaddress(const JSONRPCRequest& request) UniValue getreceivedbylabel(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -863,7 +886,9 @@ UniValue getreceivedbylabel(const JSONRPCRequest& request) UniValue getbalance(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -950,7 +975,9 @@ UniValue getbalance(const JSONRPCRequest& request) UniValue getunconfirmedbalance(const JSONRPCRequest &request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -972,7 +999,9 @@ UniValue getunconfirmedbalance(const JSONRPCRequest &request) UniValue movecmd(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -1029,7 +1058,9 @@ UniValue movecmd(const JSONRPCRequest& request) UniValue sendfrom(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -1113,7 +1144,9 @@ UniValue sendfrom(const JSONRPCRequest& request) UniValue sendmany(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -1316,7 +1349,9 @@ UniValue sendmany(const JSONRPCRequest& request) UniValue addmultisigaddress(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -1555,7 +1590,9 @@ UniValue ListReceived(CWallet * const pwallet, const UniValue& params, bool by_l UniValue listreceivedbyaddress(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -1607,7 +1644,9 @@ UniValue listreceivedbyaddress(const JSONRPCRequest& request) UniValue listreceivedbylabel(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -1779,7 +1818,9 @@ void AcentryToJSON(const CAccountingEntry& acentry, const std::string& strAccoun UniValue listtransactions(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -1980,7 +2021,9 @@ UniValue listtransactions(const JSONRPCRequest& request) UniValue listaccounts(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -2077,7 +2120,9 @@ UniValue listaccounts(const JSONRPCRequest& request) UniValue listsinceblock(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -2217,7 +2262,9 @@ UniValue listsinceblock(const JSONRPCRequest& request) UniValue gettransaction(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -2312,7 +2359,9 @@ UniValue gettransaction(const JSONRPCRequest& request) UniValue abandontransaction(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -2356,7 +2405,9 @@ UniValue abandontransaction(const JSONRPCRequest& request) UniValue backupwallet(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -2389,7 +2440,9 @@ UniValue backupwallet(const JSONRPCRequest& request) UniValue keypoolrefill(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -2436,7 +2489,9 @@ static void LockWallet(CWallet* pWallet) UniValue walletpassphrase(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -2515,7 +2570,9 @@ UniValue walletpassphrase(const JSONRPCRequest& request) UniValue walletpassphrasechange(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -2564,7 +2621,9 @@ UniValue walletpassphrasechange(const JSONRPCRequest& request) UniValue walletlock(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -2602,7 +2661,9 @@ UniValue walletlock(const JSONRPCRequest& request) UniValue encryptwallet(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -2662,7 +2723,9 @@ UniValue encryptwallet(const JSONRPCRequest& request) UniValue lockunspent(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -2789,7 +2852,9 @@ UniValue lockunspent(const JSONRPCRequest& request) UniValue listlockunspent(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -2840,7 +2905,9 @@ UniValue listlockunspent(const JSONRPCRequest& request) UniValue settxfee(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -2869,7 +2936,8 @@ UniValue settxfee(const JSONRPCRequest& request) UniValue setcoinjoinrounds(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) return NullUniValue; @@ -2897,7 +2965,8 @@ UniValue setcoinjoinrounds(const JSONRPCRequest& request) UniValue setcoinjoinamount(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) return NullUniValue; @@ -2925,7 +2994,8 @@ UniValue setcoinjoinamount(const JSONRPCRequest& request) UniValue getwalletinfo(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -3047,14 +3117,14 @@ UniValue listwallets(const JSONRPCRequest& request) UniValue obj(UniValue::VARR); - for (CWallet* pwallet : GetWallets()) { - if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { + for (const std::shared_ptr& wallet : GetWallets()) { + if (!EnsureWalletIsAvailable(wallet.get(), request.fHelp)) { return NullUniValue; } - LOCK(pwallet->cs_wallet); + LOCK(wallet->cs_wallet); - obj.push_back(pwallet->GetName()); + obj.push_back(wallet->GetName()); } return obj; @@ -3062,7 +3132,9 @@ UniValue listwallets(const JSONRPCRequest& request) UniValue upgradetohd(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -3168,7 +3240,8 @@ UniValue upgradetohd(const JSONRPCRequest& request) UniValue keepass(const JSONRPCRequest& request) { - CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) return NullUniValue; @@ -3258,7 +3331,7 @@ UniValue loadwallet(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_ERROR, "Wallet file verification failed: " + error); } - CWallet * const wallet = CWallet::CreateWalletFromFile(wallet_file, fs::absolute(wallet_file, GetWalletDir())); + std::shared_ptr const wallet = CWallet::CreateWalletFromFile(wallet_file, fs::absolute(wallet_file, GetWalletDir())); if (!wallet) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet loading failed."); } @@ -3304,7 +3377,7 @@ UniValue createwallet(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_ERROR, "Wallet file verification failed: " + error); } - CWallet* const wallet = CWallet::CreateWalletFromFile(wallet_name, fs::absolute(wallet_name, GetWalletDir())); + const auto wallet = CWallet::CreateWalletFromFile(wallet_name, fs::absolute(wallet_name, GetWalletDir())); if (!wallet) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet creation failed."); } @@ -3321,7 +3394,9 @@ UniValue createwallet(const JSONRPCRequest& request) UniValue resendwallettransactions(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -3357,7 +3432,9 @@ UniValue resendwallettransactions(const JSONRPCRequest& request) UniValue listunspent(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -3555,7 +3632,9 @@ UniValue listunspent(const JSONRPCRequest& request) UniValue fundrawtransaction(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -3730,7 +3809,9 @@ UniValue fundrawtransaction(const JSONRPCRequest& request) UniValue signrawtransactionwithwallet(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -3801,7 +3882,9 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request) #if ENABLE_MINER UniValue generate(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; @@ -3852,7 +3935,9 @@ UniValue generate(const JSONRPCRequest& request) UniValue rescanblockchain(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -3998,7 +4083,9 @@ static UniValue AddressBookDataToJSON(const CAddressBookData& data, const bool v UniValue getaddressinfo(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -4112,7 +4199,9 @@ UniValue getaddressinfo(const JSONRPCRequest& request) UniValue getaddressesbylabel(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } @@ -4155,7 +4244,9 @@ UniValue getaddressesbylabel(const JSONRPCRequest& request) UniValue listlabels(const JSONRPCRequest& request) { - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); + std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); + CWallet* const pwallet = wallet.get(); + if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) { return NullUniValue; } diff --git a/src/wallet/rpcwallet.h b/src/wallet/rpcwallet.h index af6d59fd7b26..4ac6ca22c114 100644 --- a/src/wallet/rpcwallet.h +++ b/src/wallet/rpcwallet.h @@ -20,7 +20,7 @@ void RegisterWalletRPCCommands(CRPCTable &t); * @param[in] request JSONRPCRequest that wishes to access a wallet * @return nullptr if no wallet should be used, or a pointer to the CWallet */ -CWallet *GetWalletForJSONRPCRequest(const JSONRPCRequest& request); +std::shared_ptr GetWalletForJSONRPCRequest(const JSONRPCRequest& request); std::string HelpRequiringPassphrase(CWallet *); void EnsureWalletIsUnlocked(CWallet *); diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp index 3ea32788040a..d2add77b8ed6 100644 --- a/src/wallet/test/coinjoin_tests.cpp +++ b/src/wallet/test/coinjoin_tests.cpp @@ -39,7 +39,7 @@ class CTransactionBuilderTestSetup : public TestChain100Setup wallet = MakeUnique("mock", WalletDatabase::CreateMock()); bool firstRun; wallet->LoadWallet(firstRun); - AddWallet(wallet.get()); + AddWallet(wallet); { LOCK(wallet->cs_wallet); wallet->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()); @@ -51,10 +51,10 @@ class CTransactionBuilderTestSetup : public TestChain100Setup ~CTransactionBuilderTestSetup() { - RemoveWallet(wallet.get()); + RemoveWallet(wallet); } - std::unique_ptr wallet; + std::shared_ptr wallet; CWalletTx& AddTxToChain(uint256 nTxHash) { @@ -116,7 +116,7 @@ BOOST_FIXTURE_TEST_CASE(CTransactionBuilderTest, CTransactionBuilderTestSetup) // Tests with single outpoint tallyItem { CompactTallyItem tallyItem = GetTallyItem({5000}); - CTransactionBuilder txBuilder(wallet.get(), tallyItem); + CTransactionBuilder txBuilder(wallet, tallyItem); BOOST_CHECK_EQUAL(txBuilder.CountOutputs(), 0); BOOST_CHECK_EQUAL(txBuilder.GetAmountInitial(), tallyItem.nAmount); @@ -153,7 +153,7 @@ BOOST_FIXTURE_TEST_CASE(CTransactionBuilderTest, CTransactionBuilderTestSetup) // Tests with multiple outpoint tallyItem { CompactTallyItem tallyItem = GetTallyItem({10000, 20000, 30000, 40000, 50000}); - CTransactionBuilder txBuilder(wallet.get(), tallyItem); + CTransactionBuilder txBuilder(wallet, tallyItem); std::vector vecOutputs; std::string strResult; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index b33db8e443ac..1fbbace78330 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -76,8 +76,8 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) // before the missing block, and success for a key whose creation time is // after. { - CWallet wallet("dummy", WalletDatabase::CreateDummy()); - AddWallet(&wallet); + std::shared_ptr wallet = std::make_shared("dummy", WalletDatabase::CreateDummy()); + AddWallet(wallet); UniValue keys; keys.setArray(); UniValue key; @@ -108,7 +108,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)); - RemoveWallet(&wallet); + RemoveWallet(wallet); } } @@ -137,36 +137,36 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // Import key into wallet and call dumpwallet to create backup file. { - CWallet wallet("dummy", WalletDatabase::CreateDummy()); - LOCK(wallet.cs_wallet); - wallet.mapKeyMetadata[coinbaseKey.GetPubKey().GetID()].nCreateTime = KEY_TIME; - wallet.AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()); + std::shared_ptr wallet = std::make_shared("dummy", WalletDatabase::CreateDummy()); + LOCK(wallet->cs_wallet); + wallet->mapKeyMetadata[coinbaseKey.GetPubKey().GetID()].nCreateTime = KEY_TIME; + wallet->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()); JSONRPCRequest request; request.params.setArray(); request.params.push_back(backup_file); - AddWallet(&wallet); + AddWallet(wallet); ::dumpwallet(request); - RemoveWallet(&wallet); + RemoveWallet(wallet); } // Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME // were scanned, and no prior blocks were scanned. { - CWallet wallet("dummy", WalletDatabase::CreateDummy()); + std::shared_ptr wallet = std::make_shared("dummy", WalletDatabase::CreateDummy()); JSONRPCRequest request; request.params.setArray(); request.params.push_back(backup_file); - AddWallet(&wallet); + AddWallet(wallet); ::importwallet(request); - RemoveWallet(&wallet); + RemoveWallet(wallet); - LOCK(wallet.cs_wallet); - BOOST_CHECK_EQUAL(wallet.mapWallet.size(), 3); + LOCK(wallet->cs_wallet); + BOOST_CHECK_EQUAL(wallet->mapWallet.size(), 3); BOOST_CHECK_EQUAL(coinbaseTxns.size(), 103); for (size_t i = 0; i < coinbaseTxns.size(); ++i) { - bool found = wallet.GetWalletTx(coinbaseTxns[i].GetHash()); + bool found = wallet->GetWalletTx(coinbaseTxns[i].GetHash()); bool expected = i >= 100; BOOST_CHECK_EQUAL(found, expected); } @@ -398,7 +398,7 @@ class CreateTransactionTestSetup : public TestChain100Setup wallet = MakeUnique("mock", WalletDatabase::CreateMock()); bool firstRun; wallet->LoadWallet(firstRun); - AddWallet(wallet.get()); + AddWallet(wallet); AddKey(*wallet, coinbaseKey); WalletRescanReserver reserver(wallet.get()); reserver.reserve(); @@ -407,10 +407,10 @@ class CreateTransactionTestSetup : public TestChain100Setup ~CreateTransactionTestSetup() { - RemoveWallet(wallet.get()); + RemoveWallet(wallet); } - std::unique_ptr wallet; + std::shared_ptr wallet; CCoinControl coinControl; template diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 78532f9063a8..64c9b8b5d941 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -46,24 +46,24 @@ #include static CCriticalSection cs_wallets; -static std::vector vpwallets GUARDED_BY(cs_wallets); +static std::vector> vpwallets GUARDED_BY(cs_wallets); -bool AddWallet(CWallet* wallet) +bool AddWallet(const std::shared_ptr& wallet) { LOCK(cs_wallets); assert(wallet); - std::vector::const_iterator i = std::find(vpwallets.begin(), vpwallets.end(), wallet); + std::vector>::const_iterator i = std::find(vpwallets.begin(), vpwallets.end(), wallet); if (i != vpwallets.end()) return false; coinJoinClientManagers.emplace(std::make_pair(wallet->GetName(), std::make_shared(*wallet))); vpwallets.push_back(wallet); return true; } -bool RemoveWallet(CWallet* wallet) +bool RemoveWallet(const std::shared_ptr& wallet) { LOCK(cs_wallets); assert(wallet); - std::vector::iterator i = std::find(vpwallets.begin(), vpwallets.end(), wallet); + std::vector>::iterator i = std::find(vpwallets.begin(), vpwallets.end(), wallet); if (i == vpwallets.end()) return false; vpwallets.erase(i); auto it = coinJoinClientManagers.find(wallet->GetName()); @@ -77,16 +77,16 @@ bool HasWallets() return !vpwallets.empty(); } -std::vector GetWallets() +std::vector> GetWallets() { LOCK(cs_wallets); return vpwallets; } -CWallet* GetWallet(const std::string& name) +std::shared_ptr GetWallet(const std::string& name) { LOCK(cs_wallets); - for (CWallet* wallet : vpwallets) { + for (const std::shared_ptr& wallet : vpwallets) { if (wallet->GetName() == name) return wallet; } return nullptr; @@ -4969,7 +4969,7 @@ bool CWallet::Verify(std::string wallet_file, bool salvage_wallet, std::string& return WalletBatch::VerifyDatabaseFile(wallet_path, warning_string, error_string); } -CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path& path) +std::shared_ptr CWallet::CreateWalletFromFile(const std::string& name, const fs::path& path) { const std::string& walletFile = name; @@ -4991,10 +4991,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path& int64_t nStart = GetTimeMillis(); bool fFirstRun = true; - // Make a temporary wallet unique pointer so memory doesn't get leaked if - // wallet creation fails. - auto temp_wallet = MakeUnique(name, WalletDatabase::Create(path)); - CWallet *walletInstance = temp_wallet.get(); + std::shared_ptr walletInstance = std::make_shared(name, WalletDatabase::Create(path)); AddWallet(walletInstance); auto error = [&](const std::string& strError) { RemoveWallet(walletInstance); @@ -5031,6 +5028,8 @@ CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path& } } + uiInterface.LoadWallet(walletInstance); + if (gArgs.GetBoolArg("-upgradewallet", fFirstRun)) { int nMaxVersion = gArgs.GetArg("-upgradewallet", 0); @@ -5169,7 +5168,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path& nStart = GetTimeMillis(); { - WalletRescanReserver reserver(walletInstance); + WalletRescanReserver reserver(walletInstance.get()); if (!reserver.reserve()) { return error(_("Failed to rescan the wallet during initialization")); } @@ -5205,10 +5204,8 @@ CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path& } } - uiInterface.LoadWallet(walletInstance); - // Register with the validation interface. It's ok to do this after rescan since we're still holding cs_main. - RegisterValidationInterface(temp_wallet.release()); + RegisterValidationInterface(walletInstance.get()); walletInstance->SetBroadcastTransactions(gArgs.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index eba1eb680e75..7cae60e5a650 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -38,12 +38,11 @@ #include #include -typedef CWallet* CWalletRef; -bool AddWallet(CWallet* wallet); -bool RemoveWallet(CWallet* wallet); +bool AddWallet(const std::shared_ptr& wallet); +bool RemoveWallet(const std::shared_ptr& wallet); bool HasWallets(); -std::vector GetWallets(); -CWallet* GetWallet(const std::string& name); +std::vector> GetWallets(); +std::shared_ptr GetWallet(const std::string& name); /** * Settings @@ -1208,7 +1207,7 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface static bool Verify(std::string wallet_file, bool salvage_wallet, std::string& error_string, std::string& warning_string); /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ - static CWallet* CreateWalletFromFile(const std::string& name, const fs::path& path); + static std::shared_ptr 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 75daf528ec43..16f3b24f152b 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -832,7 +832,7 @@ void MaybeCompactWalletDB() return; } - for (CWallet* pwallet : GetWallets()) { + for (const std::shared_ptr& pwallet : GetWallets()) { WalletDatabase& dbh = pwallet->GetDBHandle(); unsigned int nUpdateCounter = dbh.nUpdateCounter;