From 537efe19e696a97ca6a4f461b2c1b4cb7ab001b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Tue, 12 Jun 2018 14:40:52 +0100 Subject: [PATCH 1/8] rpc: Extract GetWalletNameFromJSONRPCRequest from GetWalletForJSONRPCRequest --- src/wallet/rpcwallet.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 456f08bc14b..d4c281b13ce 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -40,12 +40,21 @@ static const std::string WALLET_ENDPOINT_BASE = "/wallet/"; -std::shared_ptr GetWalletForJSONRPCRequest(const JSONRPCRequest& request) +bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& wallet_name) { 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())); - std::shared_ptr pwallet = GetWallet(requestedWallet); + wallet_name = urlDecode(request.URI.substr(WALLET_ENDPOINT_BASE.size())); + return true; + } + return false; +} + +std::shared_ptr GetWalletForJSONRPCRequest(const JSONRPCRequest& request) +{ + std::string wallet_name; + if (GetWalletNameFromJSONRPCRequest(request, wallet_name)) { + std::shared_ptr pwallet = GetWallet(wallet_name); if (!pwallet) throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Requested wallet does not exist or is not loaded"); return pwallet; } From 6608c369b1a6cfc1d5b4a7905c193baa999ba84c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Sat, 28 Apr 2018 22:36:43 +0100 Subject: [PATCH 2/8] rpc: Add unloadwallet RPC --- src/wallet/rpcwallet.cpp | 54 ++++++++++++++++++++++++++++++++++++++-- src/wallet/wallet.cpp | 15 +++++++++-- src/wallet/wallet.h | 3 +++ 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index d4c281b13ce..daef8473f1f 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3076,7 +3076,7 @@ static UniValue listwallets(const JSONRPCRequest& request) return obj; } -UniValue loadwallet(const JSONRPCRequest& request) +static UniValue loadwallet(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() != 1) throw std::runtime_error( @@ -3123,7 +3123,7 @@ UniValue loadwallet(const JSONRPCRequest& request) return obj; } -UniValue createwallet(const JSONRPCRequest& request) +static UniValue createwallet(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() != 1) { throw std::runtime_error( @@ -3170,6 +3170,55 @@ UniValue createwallet(const JSONRPCRequest& request) return obj; } +static UniValue unloadwallet(const JSONRPCRequest& request) +{ + if (request.fHelp || request.params.size() > 1) { + throw std::runtime_error( + "unloadwallet ( \"wallet_name\" )\n" + "Unloads the wallet referenced by the request endpoint otherwise unloads the wallet specified in the argument.\n" + "Specifying the wallet name on a wallet endpoint is invalid." + "\nArguments:\n" + "1. \"wallet_name\" (string, optional) The name of the wallet to unload.\n" + "\nExamples:\n" + + HelpExampleCli("unloadwallet", "wallet_name") + + HelpExampleRpc("unloadwallet", "wallet_name") + ); + } + + std::string wallet_name; + if (GetWalletNameFromJSONRPCRequest(request, wallet_name)) { + if (!request.params[0].isNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot unload the requested wallet"); + } + } else { + wallet_name = request.params[0].get_str(); + } + + std::shared_ptr wallet = GetWallet(wallet_name); + if (!wallet) { + throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Requested wallet does not exist or is not loaded"); + } + + // Release the "main" shared pointer and prevent further notifications. + // Note that any attempt to load the same wallet would fail until the wallet + // is destroyed (see CheckUniqueFileid). + if (!RemoveWallet(wallet)) { + throw JSONRPCError(RPC_MISC_ERROR, "Requested wallet already unloaded"); + } + UnregisterValidationInterface(wallet.get()); + + // The wallet can be in use so it's not possible to explicitly unload here. + // Just notify the unload intent so that all shared pointers are released. + // The wallet will be destroyed once the last shared pointer is released. + wallet->NotifyUnload(); + + // There's no point in waiting for the wallet to unload. + // At this point this method should never fail. The unloading could only + // fail due to an unexpected error which would cause a process termination. + + return NullUniValue; +} + static UniValue resendwallettransactions(const JSONRPCRequest& request) { std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); @@ -4405,6 +4454,7 @@ static const CRPCCommand commands[] = { "wallet", "settxfee", &settxfee, {"amount"} }, { "wallet", "signmessage", &signmessage, {"address","message"} }, { "wallet", "signrawtransactionwithwallet", &signrawtransactionwithwallet, {"hexstring","prevtxs","sighashtype"} }, + { "wallet", "unloadwallet", &unloadwallet, {"wallet_name"} }, { "wallet", "walletlock", &walletlock, {} }, { "wallet", "walletpassphrasechange", &walletpassphrasechange, {"oldpassphrase","newpassphrase"} }, { "wallet", "walletpassphrase", &walletpassphrase, {"passphrase","timeout"} }, diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c3597aace83..a3bda89468a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -79,6 +79,15 @@ std::shared_ptr GetWallet(const std::string& name) return nullptr; } +// Custom deleter for shared_ptr. +static void ReleaseWallet(CWallet* wallet) +{ + LogPrintf("Releasing wallet %s\n", wallet->GetName()); + wallet->BlockUntilSyncedToCurrentChain(); + wallet->Flush(); + delete wallet; +} + const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; const uint256 CMerkleTx::ABANDON_HASH(uint256S("0000000000000000000000000000000000000000000000000000000000000001")); @@ -1294,7 +1303,7 @@ void CWallet::BlockUntilSyncedToCurrentChain() { LOCK(cs_main); const CBlockIndex* initialChainTip = chainActive.Tip(); - if (m_last_block_processed->GetAncestor(initialChainTip->nHeight) == initialChainTip) { + if (m_last_block_processed && m_last_block_processed->GetAncestor(initialChainTip->nHeight) == initialChainTip) { return; } } @@ -4057,7 +4066,9 @@ std::shared_ptr CWallet::CreateWalletFromFile(const std::string& name, int64_t nStart = GetTimeMillis(); bool fFirstRun = true; - std::shared_ptr walletInstance = std::make_shared(name, WalletDatabase::Create(path)); + // TODO: Can't use std::make_shared because we need a custom deleter but + // should be possible to use std::allocate_shared. + std::shared_ptr walletInstance(new CWallet(name, WalletDatabase::Create(path)), ReleaseWallet); DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); if (nLoadWalletRet != DBErrors::LOAD_OK) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index f1761b0fcff..825209c143e 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1097,6 +1097,9 @@ public: //! Flush wallet (bitdb flush) void Flush(bool shutdown=false); + /** Wallet is about to be unloaded */ + boost::signals2::signal NotifyUnload; + /** * Address book entry changed. * @note called with lock cs_wallet held. From 4940a20a46685cd56ea045d8cc7fe058c6222431 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Fri, 1 Jun 2018 23:24:29 +0100 Subject: [PATCH 3/8] test: Add functional tests for unloadwallet RPC --- test/functional/wallet_multiwallet.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 53638615f6c..7f739a13161 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -234,5 +234,26 @@ class MultiWalletTest(BitcoinTestFramework): assert new_wallet_name in self.nodes[0].listwallets() + self.log.info("Test dynamic wallet unloading") + + # Test `unloadwallet` errors + assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].unloadwallet) + assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", self.nodes[0].unloadwallet, "dummy") + assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", node.get_wallet_rpc("dummy").unloadwallet) + assert_raises_rpc_error(-8, "Cannot unload the requested wallet", w1.unloadwallet, "w2"), + + # Successfully unload the specified wallet name + self.nodes[0].unloadwallet("w1") + assert 'w1' not in self.nodes[0].listwallets() + + # Successfully unload the wallet referenced by the request endpoint + w2.unloadwallet() + assert 'w2' not in self.nodes[0].listwallets() + + # Successfully unload all wallets + for wallet_name in self.nodes[0].listwallets(): + self.nodes[0].unloadwallet(wallet_name) + assert_equal(self.nodes[0].listwallets(), []) + if __name__ == '__main__': MultiWalletTest().main() From ccbf7ae7496fd13b4147aa13d7408712bd90c614 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Tue, 12 Jun 2018 15:30:32 +0100 Subject: [PATCH 4/8] test: Wallet methods are disabled when no wallet is loaded --- src/wallet/rpcwallet.cpp | 5 ----- test/functional/wallet_multiwallet.py | 1 + 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index daef8473f1f..5e9eb2b26ad 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -75,11 +75,6 @@ bool EnsureWalletIsAvailable(CWallet * const pwallet, bool avoidException) if (pwallet) return true; if (avoidException) return false; 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 - // the future that wallet RPC methods might get or remain registered - // when no wallets are loaded. throw JSONRPCError( RPC_METHOD_NOT_FOUND, "Method not found (wallet method is disabled because no wallet is loaded)"); } diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 7f739a13161..8d15ef661b4 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -254,6 +254,7 @@ class MultiWalletTest(BitcoinTestFramework): for wallet_name in self.nodes[0].listwallets(): self.nodes[0].unloadwallet(wallet_name) assert_equal(self.nodes[0].listwallets(), []) + assert_raises_rpc_error(-32601, "Method not found (wallet method is disabled because no wallet is loaded)", self.nodes[0].getwalletinfo) if __name__ == '__main__': MultiWalletTest().main() From 9f9b50d5feb1e604283c463e289e83b63a849a8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Fri, 1 Jun 2018 23:23:45 +0100 Subject: [PATCH 5/8] doc: Add release notes for unloadwallet RPC --- doc/release-notes-pr10740.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/release-notes-pr10740.md b/doc/release-notes-pr10740.md index c81ea6a4db4..a2eb8cd8370 100644 --- a/doc/release-notes-pr10740.md +++ b/doc/release-notes-pr10740.md @@ -1,9 +1,10 @@ Dynamic loading and creation of wallets --------------------------------------- -Previously, wallets could only be loaded or created at startup, by specifying `-wallet` parameters on the command line or in the bitcoin.conf file. It is now possible to load and create wallets dynamically at runtime: +Previously, wallets could only be loaded or created at startup, by specifying `-wallet` parameters on the command line or in the bitcoin.conf file. It is now possible to load, create and unload wallets dynamically at runtime: - Existing wallets can be loaded by calling the `loadwallet` RPC. The wallet can be specified as file/directory basename (which must be located in the `walletdir` directory), or as an absolute path to a file/directory. - New wallets can be created (and loaded) by calling the `createwallet` RPC. The provided name must not match a wallet file in the `walletdir` directory or the name of a wallet that is currently loaded. +- Loaded wallets can be unloaded by calling the `unloadwallet` RPC. This feature is currently only available through the RPC interface. From 0ee77b20771fe34f8dbde6b16d7e2637859baec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Tue, 5 Jun 2018 11:17:28 +0100 Subject: [PATCH 6/8] ui: Support wallets unloaded dynamically --- src/interfaces/wallet.cpp | 4 ++++ src/interfaces/wallet.h | 4 ++++ src/qt/bitcoin.cpp | 18 ++++++++++++++++-- src/qt/bitcoingui.cpp | 34 +++++++++++++++++++++++++++++----- src/qt/bitcoingui.h | 5 ++++- src/qt/rpcconsole.cpp | 10 ++++++++++ src/qt/rpcconsole.h | 1 + src/qt/walletmodel.cpp | 8 ++++++++ src/qt/walletmodel.h | 4 ++++ 9 files changed, 80 insertions(+), 8 deletions(-) diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 3029dbe8e37..e98acba0df1 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -429,6 +429,10 @@ public: bool hdEnabled() override { return m_wallet.IsHDEnabled(); } OutputType getDefaultAddressType() override { return m_wallet.m_default_address_type; } OutputType getDefaultChangeType() override { return m_wallet.m_default_change_type; } + std::unique_ptr handleUnload(UnloadFn fn) override + { + return MakeHandler(m_wallet.NotifyUnload.connect(fn)); + } std::unique_ptr handleShowProgress(ShowProgressFn fn) override { return MakeHandler(m_wallet.ShowProgress.connect(fn)); diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 82ae0b14b54..ce42e14eea8 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -242,6 +242,10 @@ public: // Get default change type. virtual OutputType getDefaultChangeType() = 0; + //! Register handler for unload message. + using UnloadFn = std::function; + virtual std::unique_ptr handleUnload(UnloadFn fn) = 0; + //! Register handler for show progress messages. using ShowProgressFn = std::function; virtual std::unique_ptr handleShowProgress(ShowProgressFn fn) = 0; diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 31d9f936e7a..6ddc8191133 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -238,6 +238,7 @@ public Q_SLOTS: /// Handle runaway exceptions. Shows a message box with the problem and quits the program. void handleRunawayException(const QString &message); void addWallet(WalletModel* walletModel); + void removeWallet(); Q_SIGNALS: void requestedInitialize(); @@ -467,11 +468,22 @@ void BitcoinApplication::addWallet(WalletModel* walletModel) connect(walletModel, SIGNAL(coinsSent(WalletModel*, SendCoinsRecipient, QByteArray)), paymentServer, SLOT(fetchPaymentACK(WalletModel*, const SendCoinsRecipient&, QByteArray))); + connect(walletModel, SIGNAL(unload()), this, SLOT(removeWallet())); m_wallet_models.push_back(walletModel); #endif } +void BitcoinApplication::removeWallet() +{ +#ifdef ENABLE_WALLET + WalletModel* walletModel = static_cast(sender()); + m_wallet_models.erase(std::find(m_wallet_models.begin(), m_wallet_models.end(), walletModel)); + window->removeWallet(walletModel); + walletModel->deleteLater(); +#endif +} + void BitcoinApplication::initializeResult(bool success) { qDebug() << __func__ << ": Initialization result: " << success; @@ -491,8 +503,10 @@ void BitcoinApplication::initializeResult(bool success) #ifdef ENABLE_WALLET m_handler_load_wallet = m_node.handleLoadWallet([this](std::unique_ptr wallet) { - QMetaObject::invokeMethod(this, "addWallet", Qt::QueuedConnection, - Q_ARG(WalletModel*, new WalletModel(std::move(wallet), m_node, platformStyle, optionsModel))); + WalletModel* wallet_model = new WalletModel(std::move(wallet), m_node, platformStyle, optionsModel, nullptr); + // Fix wallet model thread affinity. + wallet_model->moveToThread(thread()); + QMetaObject::invokeMethod(this, "addWallet", Qt::QueuedConnection, Q_ARG(WalletModel*, wallet_model)); }); for (auto& wallet : m_node.getWallets()) { diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index 9f5ea02e148..c6cc6312016 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -120,6 +120,7 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const PlatformStyle *_platformSty modalOverlay(0), prevBlocks(0), spinnerFrame(0), + m_wallet_selector_label(nullptr), platformStyle(_platformStyle) { QSettings settings; @@ -477,6 +478,16 @@ void BitcoinGUI::createToolBars() m_wallet_selector = new QComboBox(); connect(m_wallet_selector, SIGNAL(currentIndexChanged(int)), this, SLOT(setCurrentWalletBySelectorIndex(int))); + + m_wallet_selector_label = new QLabel(); + m_wallet_selector_label->setText(tr("Wallet:") + " "); + m_wallet_selector_label->setBuddy(m_wallet_selector); + + m_wallet_selector_label_action = appToolBar->addWidget(m_wallet_selector_label); + m_wallet_selector_action = appToolBar->addWidget(m_wallet_selector); + + m_wallet_selector_label_action->setVisible(false); + m_wallet_selector_action->setVisible(false); #endif } } @@ -556,16 +567,29 @@ bool BitcoinGUI::addWallet(WalletModel *walletModel) setWalletActionsEnabled(true); m_wallet_selector->addItem(display_name, name); if (m_wallet_selector->count() == 2) { - m_wallet_selector_label = new QLabel(); - m_wallet_selector_label->setText(tr("Wallet:") + " "); - m_wallet_selector_label->setBuddy(m_wallet_selector); - appToolBar->addWidget(m_wallet_selector_label); - appToolBar->addWidget(m_wallet_selector); + m_wallet_selector_label_action->setVisible(true); + m_wallet_selector_action->setVisible(true); } rpcConsole->addWallet(walletModel); return walletFrame->addWallet(walletModel); } +bool BitcoinGUI::removeWallet(WalletModel* walletModel) +{ + if (!walletFrame) return false; + QString name = walletModel->getWalletName(); + int index = m_wallet_selector->findData(name); + m_wallet_selector->removeItem(index); + if (m_wallet_selector->count() == 0) { + setWalletActionsEnabled(false); + } else if (m_wallet_selector->count() == 1) { + m_wallet_selector_label_action->setVisible(false); + m_wallet_selector_action->setVisible(false); + } + rpcConsole->removeWallet(walletModel); + return walletFrame->removeWallet(name); +} + bool BitcoinGUI::setCurrentWallet(const QString& name) { if(!walletFrame) diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index 89c1c73a79f..68c35557cc5 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -70,6 +70,7 @@ public: functionality. */ bool addWallet(WalletModel *walletModel); + bool removeWallet(WalletModel* walletModel); void removeAllWallets(); #endif // ENABLE_WALLET bool enableWallet; @@ -122,8 +123,10 @@ private: QAction *openRPCConsoleAction; QAction *openAction; QAction *showHelpMessageAction; + QAction *m_wallet_selector_label_action = nullptr; + QAction *m_wallet_selector_action = nullptr; - QLabel *m_wallet_selector_label; + QLabel *m_wallet_selector_label = nullptr; QComboBox *m_wallet_selector; QSystemTrayIcon *trayIcon; diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 4550ae9396b..e4e8d3535a0 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -713,6 +713,16 @@ void RPCConsole::addWallet(WalletModel * const walletModel) ui->WalletSelectorLabel->setVisible(true); } } + +void RPCConsole::removeWallet(WalletModel * const walletModel) +{ + const QString name = walletModel->getWalletName(); + ui->WalletSelector->removeItem(ui->WalletSelector->findData(name)); + if (ui->WalletSelector->count() == 2) { + ui->WalletSelector->setVisible(false); + ui->WalletSelectorLabel->setVisible(false); + } +} #endif static QString categoryClass(int category) diff --git a/src/qt/rpcconsole.h b/src/qt/rpcconsole.h index a53c4c24f91..0a1a4699341 100644 --- a/src/qt/rpcconsole.h +++ b/src/qt/rpcconsole.h @@ -48,6 +48,7 @@ public: void setClientModel(ClientModel *model); void addWallet(WalletModel * const walletModel); + void removeWallet(WalletModel* const walletModel); enum MessageClass { MC_ERROR, diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 3418b1f1a9c..389acf0a95a 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -364,6 +364,12 @@ bool WalletModel::changePassphrase(const SecureString &oldPass, const SecureStri } // Handlers for core signals +static void NotifyUnload(WalletModel* walletModel) +{ + qDebug() << "NotifyUnload"; + QMetaObject::invokeMethod(walletModel, "unload", Qt::QueuedConnection); +} + static void NotifyKeyStoreStatusChanged(WalletModel *walletmodel) { qDebug() << "NotifyKeyStoreStatusChanged"; @@ -411,6 +417,7 @@ static void NotifyWatchonlyChanged(WalletModel *walletmodel, bool fHaveWatchonly void WalletModel::subscribeToCoreSignals() { // Connect signals to wallet + m_handler_unload = m_wallet->handleUnload(boost::bind(&NotifyUnload, this)); m_handler_status_changed = m_wallet->handleStatusChanged(boost::bind(&NotifyKeyStoreStatusChanged, this)); m_handler_address_book_changed = m_wallet->handleAddressBookChanged(boost::bind(NotifyAddressBookChanged, this, _1, _2, _3, _4, _5)); m_handler_transaction_changed = m_wallet->handleTransactionChanged(boost::bind(NotifyTransactionChanged, this, _1, _2)); @@ -421,6 +428,7 @@ void WalletModel::subscribeToCoreSignals() void WalletModel::unsubscribeFromCoreSignals() { // Disconnect signals from wallet + m_handler_unload->disconnect(); m_handler_status_changed->disconnect(); m_handler_address_book_changed->disconnect(); m_handler_transaction_changed->disconnect(); diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 9173fcae52d..35ededb1210 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -208,6 +208,7 @@ public: AddressTableModel* getAddressTableModel() const { return addressTableModel; } private: std::unique_ptr m_wallet; + std::unique_ptr m_handler_unload; std::unique_ptr m_handler_status_changed; std::unique_ptr m_handler_address_book_changed; std::unique_ptr m_handler_transaction_changed; @@ -261,6 +262,9 @@ Q_SIGNALS: // Watch-only address added void notifyWatchonlyChanged(bool fHaveWatchonly); + // Signal that wallet is about to be removed + void unload(); + public Q_SLOTS: /* Wallet status might have changed */ void updateStatus(); From 0b82bac76d0f842bd2294a290388536951fbc576 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 7/8] bugfix: Remove dangling wallet env instance --- src/wallet/db.cpp | 5 ++++- test/functional/wallet_multiwallet.py | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 410dd5009f2..01b8eacccb8 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -694,8 +694,10 @@ void BerkeleyEnvironment::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); } } } @@ -794,5 +796,6 @@ void BerkeleyDatabase::Flush(bool shutdown) { if (!IsDummy()) { env->Flush(shutdown); + if (shutdown) env = nullptr; } } diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 8d15ef661b4..e6097b5d92c 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -256,5 +256,10 @@ class MultiWalletTest(BitcoinTestFramework): assert_equal(self.nodes[0].listwallets(), []) assert_raises_rpc_error(-32601, "Method not found (wallet method is disabled because no wallet is loaded)", self.nodes[0].getwalletinfo) + # Successfully load a previously unloaded wallet + self.nodes[0].loadwallet('w1') + assert_equal(self.nodes[0].listwallets(), ['w1']) + assert_equal(w1.getwalletinfo()['walletname'], 'w1') + if __name__ == '__main__': MultiWalletTest().main() From fe65bdec237776dbe094339509dfd2e63329a832 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Wed, 20 Jun 2018 14:15:12 +0100 Subject: [PATCH 8/8] bugfix: Delete walletView in WalletFrame::removeWallet --- src/qt/walletframe.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index eb0eba21ef7..c5a13f61f4a 100644 --- a/src/qt/walletframe.cpp +++ b/src/qt/walletframe.cpp @@ -94,6 +94,7 @@ bool WalletFrame::removeWallet(const QString &name) WalletView *walletView = mapWalletViews.take(name); walletStack->removeWidget(walletView); + delete walletView; return true; }