diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -7,3 +7,5 @@ the bitcoin-qt GUI. - The default wallet will now be labeled `[default wallet]` in the bitcoin-qt GUI if no name is provided by the `-wallet` option on start up. + - It is now possible to unload wallets dynamically at runtime. This feature is + currently only available through the RPC interface. diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -96,11 +96,6 @@ 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)"); @@ -3361,7 +3356,8 @@ return obj; } -UniValue loadwallet(const Config &config, const JSONRPCRequest &request) { +static UniValue loadwallet(const Config &config, + const JSONRPCRequest &request) { if (request.fHelp || request.params.size() != 1) { throw std::runtime_error( "loadwallet \"filename\"\n" @@ -3427,7 +3423,8 @@ return obj; } -UniValue createwallet(const Config &config, const JSONRPCRequest &request) { +static UniValue createwallet(const Config &config, + const JSONRPCRequest &request) { if (request.fHelp || request.params.size() != 1) { throw std::runtime_error( "createwallet \"wallet_name\"\n" @@ -3484,6 +3481,58 @@ return obj; } +static UniValue unloadwallet(const Config &config, + 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 Config &config, const JSONRPCRequest &request) { std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); @@ -4576,6 +4625,7 @@ { "wallet", "settxfee", settxfee, {"amount"} }, { "wallet", "signmessage", signmessage, {"address","message"} }, { "wallet", "signrawtransactionwithwallet", signrawtransactionwithwallet, {"hextring","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.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1224,6 +1224,9 @@ //! 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. diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -88,6 +88,14 @@ 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; +} + static const size_t OUTPUT_GROUP_MAX_ENTRIES = 10; const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; @@ -1367,8 +1375,9 @@ 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; } } @@ -4364,8 +4373,11 @@ int64_t nStart = GetTimeMillis(); bool fFirstRun = true; - std::shared_ptr walletInstance = std::make_shared( - chainParams, 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(chainParams, name, WalletDatabase::Create(path)), + ReleaseWallet); DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); if (nLoadWalletRet != DBErrors::LOAD_OK) { if (nLoadWalletRet == DBErrors::CORRUPT) { diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -251,6 +251,38 @@ 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(), []) + 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()