Skip to content

Commit abbb7ec

Browse files
committed
refactor: Move restorewallet() RPC logic to the wallet section
It also simplifies restorewallet() and loadwallet() RPC error handling.
1 parent 4807f73 commit abbb7ec

File tree

8 files changed

+34
-33
lines changed

8 files changed

+34
-33
lines changed

src/rpc/protocol.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ enum RPCErrorCode
8080
RPC_WALLET_NOT_FOUND = -18, //!< Invalid wallet specified
8181
RPC_WALLET_NOT_SPECIFIED = -19, //!< No wallet specified (error when there are multiple wallets loaded)
8282
RPC_WALLET_ALREADY_LOADED = -35, //!< This same wallet is already loaded
83+
RPC_WALLET_ALREADY_EXISTS = -36, //!< There is already a wallet with the same name
8384

8485
//! Backwards compatible aliases
8586
RPC_WALLET_INVALID_ACCOUNT_NAME = RPC_WALLET_INVALID_LABEL_NAME,

src/wallet/db.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ enum class DatabaseStatus {
220220
FAILED_LOAD,
221221
FAILED_VERIFY,
222222
FAILED_ENCRYPT,
223+
FAILED_INVALID_BACKUP_FILE,
223224
};
224225

225226
/** Recursively list database paths in directory. */

src/wallet/rpc/backup.cpp

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1879,27 +1879,17 @@ RPCHelpMan restorewallet()
18791879

18801880
auto backup_file = fs::u8path(request.params[1].get_str());
18811881

1882-
if (!fs::exists(backup_file)) {
1883-
throw JSONRPCError(RPC_INVALID_PARAMETER, "Backup file does not exist");
1884-
}
1885-
18861882
std::string wallet_name = request.params[0].get_str();
18871883

1888-
const fs::path wallet_path = fsbridge::AbsPathJoin(GetWalletDir(), fs::u8path(wallet_name));
1889-
1890-
if (fs::exists(wallet_path)) {
1891-
throw JSONRPCError(RPC_INVALID_PARAMETER, "Wallet name already exists.");
1892-
}
1893-
1894-
if (!TryCreateDirectories(wallet_path)) {
1895-
throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Failed to create database path '%s'. Database already exists.", wallet_path.u8string()));
1896-
}
1884+
std::optional<bool> load_on_start = request.params[2].isNull() ? std::nullopt : std::optional<bool>(request.params[2].get_bool());
18971885

1898-
auto wallet_file = wallet_path / "wallet.dat";
1886+
DatabaseStatus status;
1887+
bilingual_str error;
1888+
std::vector<bilingual_str> warnings;
18991889

1900-
fs::copy_file(backup_file, wallet_file, fs::copy_option::fail_if_exists);
1890+
const std::shared_ptr<CWallet> wallet = RestoreWallet(context, fs::PathToString(backup_file), wallet_name, load_on_start, status, error, warnings);
19011891

1902-
auto [wallet, warnings] = LoadWalletHelper(context, request.params[2], wallet_name);
1892+
HandleWalletError(wallet, status, error);
19031893

19041894
UniValue obj(UniValue::VOBJ);
19051895
obj.pushKV("name", wallet->GetName());

src/wallet/rpc/util.cpp

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -122,16 +122,8 @@ std::string LabelFromValue(const UniValue& value)
122122
return label;
123123
}
124124

125-
std::tuple<std::shared_ptr<CWallet>, std::vector<bilingual_str>> LoadWalletHelper(WalletContext& context, UniValue load_on_start_param, const std::string wallet_name)
125+
void HandleWalletError(const std::shared_ptr<CWallet> wallet, DatabaseStatus& status, bilingual_str& error)
126126
{
127-
DatabaseOptions options;
128-
DatabaseStatus status;
129-
options.require_existing = true;
130-
bilingual_str error;
131-
std::vector<bilingual_str> warnings;
132-
std::optional<bool> load_on_start = load_on_start_param.isNull() ? std::nullopt : std::optional<bool>(load_on_start_param.get_bool());
133-
std::shared_ptr<CWallet> const wallet = LoadWallet(context, wallet_name, load_on_start, options, status, error, warnings);
134-
135127
if (!wallet) {
136128
// Map bad format to not found, since bad format is returned when the
137129
// wallet directory exists, but doesn't contain a data file.
@@ -144,11 +136,15 @@ std::tuple<std::shared_ptr<CWallet>, std::vector<bilingual_str>> LoadWalletHelpe
144136
case DatabaseStatus::FAILED_ALREADY_LOADED:
145137
code = RPC_WALLET_ALREADY_LOADED;
146138
break;
139+
case DatabaseStatus::FAILED_ALREADY_EXISTS:
140+
code = RPC_WALLET_ALREADY_EXISTS;
141+
break;
142+
case DatabaseStatus::FAILED_INVALID_BACKUP_FILE:
143+
code = RPC_INVALID_PARAMETER;
144+
break;
147145
default: // RPC_WALLET_ERROR is returned for all other cases.
148146
break;
149147
}
150148
throw JSONRPCError(code, error.original);
151149
}
152-
153-
return { wallet, warnings };
154-
}
150+
}

src/wallet/rpc/util.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
struct bilingual_str;
1414
class CWallet;
15+
enum class DatabaseStatus;
1516
class JSONRPCRequest;
1617
class LegacyScriptPubKeyMan;
1718
class UniValue;
@@ -37,6 +38,6 @@ bool GetAvoidReuseFlag(const CWallet& wallet, const UniValue& param);
3738
bool ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& wallet);
3839
std::string LabelFromValue(const UniValue& value);
3940

40-
std::tuple<std::shared_ptr<CWallet>, std::vector<bilingual_str>> LoadWalletHelper(WalletContext& context, UniValue load_on_start_param, const std::string wallet_name);
41+
void HandleWalletError(const std::shared_ptr<CWallet> wallet, DatabaseStatus& status, bilingual_str& error);
4142

4243
#endif // BITCOIN_WALLET_RPC_UTIL_H

src/wallet/rpc/wallet.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,15 @@ static RPCHelpMan loadwallet()
215215
WalletContext& context = EnsureWalletContext(request.context);
216216
const std::string name(request.params[0].get_str());
217217

218-
auto [wallet, warnings] = LoadWalletHelper(context, request.params[1], name);
218+
DatabaseOptions options;
219+
DatabaseStatus status;
220+
options.require_existing = true;
221+
bilingual_str error;
222+
std::vector<bilingual_str> warnings;
223+
std::optional<bool> load_on_start = request.params[1].isNull() ? std::nullopt : std::optional<bool>(request.params[1].get_bool());
224+
std::shared_ptr<CWallet> const wallet = LoadWallet(context, name, load_on_start, options, status, error, warnings);
225+
226+
HandleWalletError(wallet, status, error);
219227

220228
UniValue obj(UniValue::VOBJ);
221229
obj.pushKV("name", wallet->GetName());

src/wallet/wallet.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const std::string
364364

365365
if (!fs::exists(fs::u8path(backup_file))) {
366366
error = Untranslated("Backup file does not exist");
367-
status = DatabaseStatus::FAILED_BAD_PATH;
367+
status = DatabaseStatus::FAILED_INVALID_BACKUP_FILE;
368368
return nullptr;
369369
}
370370

test/functional/wallet_backup.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,16 @@ def restore_nonexistent_wallet(self):
115115
nonexistent_wallet_file = os.path.join(self.nodes[0].datadir, 'nonexistent_wallet.bak')
116116
wallet_name = "res0"
117117
assert_raises_rpc_error(-8, "Backup file does not exist", node.restorewallet, wallet_name, nonexistent_wallet_file)
118+
not_created_wallet_file = os.path.join(node.datadir, self.chain, 'wallets', wallet_name)
119+
assert not os.path.exists(not_created_wallet_file)
118120

119121
def restore_wallet_existent_name(self):
120122
node = self.nodes[3]
121-
wallet_file = os.path.join(self.nodes[0].datadir, 'wallet.bak')
123+
backup_file = os.path.join(self.nodes[0].datadir, 'wallet.bak')
122124
wallet_name = "res0"
123-
assert_raises_rpc_error(-8, "Wallet name already exists.", node.restorewallet, wallet_name, wallet_file)
125+
wallet_file = os.path.join(node.datadir, self.chain, 'wallets', wallet_name)
126+
error_message = "Failed to create database path '{}'. Database already exists.".format(wallet_file)
127+
assert_raises_rpc_error(-36, error_message, node.restorewallet, wallet_name, backup_file)
124128

125129
def init_three(self):
126130
self.init_wallet(node=0)

0 commit comments

Comments
 (0)