[bitcoin/bitcoin] [wallet] `loadwallet` RPC — load wallet at runtime (#10740)

TheBlueMatt commented on this pull request.

Should probably fix the various memory leaks in CreateWalletFromFile first — we didnt really use to care cause it would just persist through operation, now we do.

> — return InitError(strprintf(_(«Error loading wallet %s. Duplicate -wallet filename specified.»), walletFile));
— }

— std::string strError;
— if (!CWalletDB::VerifyEnvironment(walletFile, GetDataDir().string(), strError)) {
— return InitError(strError);
— }

— if (gArgs.GetBoolArg(«-salvagewallet», false)) {
— // Recover readable keypairs:
— CWallet dummyWallet;
— std::string backup_filename;
— if (!CWalletDB::Recover(walletFile, (void *)&dummyWallet, CWalletDB::RecoverKeysOnlyFilter, backup_filename)) {
— return false;
— }
+ return InitError(strprintf(_(«Error loading wallet %s. Duplicate wallet filename specified.»), walletFile));

This is redundant now, no?

> @@ -1080,6 +1080,9 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface
/** Mark a transaction as replaced by another transaction (e.g., BIP 125). */
bool MarkReplaced(const uint256& originalHash, const uint256& newHash);

+ //! Verify wallet naming and perform salvage on the wallet if required
+ static bool VerifyWallet(std::string walletFile, bool salvage_wallet);

nit: I’d rather not make this a static CWallet member. wallet.cpp is already plenty big and the stuff that it does seems more external db-related stuff and less wallet-related stuff. Ideally it’d be nice to more clearly separate the DB stuff from the wallet logic itself by pulling things like the DB-related stuff out of CreateWalletFromFile.

> + » «name» : , (string) The wallet name if loaded successfully.n»
+ «}n»
+ «nExamples:n»
+ + HelpExampleCli(«loadwallet», «»test.dat»»)
+ + HelpExampleRpc(«loadwallet», «»test.dat»»)
+ );
+ std::string wallet_file = request.params[0].get_str();
+ std::string error;
+
+ fs::path wallet_path = fs::absolute(wallet_file, GetWalletDir());
+ if (fs::symlink_status(wallet_path).type() == fs::file_not_found) {
+ throw JSONRPCError(RPC_WALLET_NOT_FOUND, «Wallet » + wallet_file + » not found.»);
+ }
+
+ std::string dummy_warning;
+ if (!CWallet::Verify(wallet_file, false, error, dummy_warning)) {

This needs some kind of lock to avoid double-loading a wallet in two rpc threads at once.

> @@ -2994,6 +2994,49 @@ static UniValue listwallets(const JSONRPCRequest& request)
return obj;
}

+UniValue loadwallet(const JSONRPCRequest& request)
+{
+ if (request.fHelp || request.params.size() != 1)
+ throw std::runtime_error(
+ «loadwallet «filename»n»
+ «nLoads a wallet from a wallet file or directory.n»

Maybe some help messages noting that certain config options still apply here — eg if you started with something like zapwallettxes, upgradewallet, rescan, etc it will take the same action for newly-loaded wallets here.

Добавить комментарий