Skip to content

Commit

Permalink
wallet: Remove path checking code from loadwallet RPC
Browse files Browse the repository at this point in the history
Summary:
This commit does not change behavior except for error messages which now
include more complete information.

This is a backport of [[bitcoin/bitcoin#19619 | core#19619]] [6/8]
bitcoin/bitcoin@a987438

Depends on D10228
Depends on D10213 (for rpc_createmultisig.py)

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10229
  • Loading branch information
ryanofsky authored and PiRK committed Oct 5, 2021
1 parent 3d15c53 commit 18aeb32
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 22 deletions.
24 changes: 8 additions & 16 deletions src/wallet/rpcwallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3128,24 +3128,10 @@ static UniValue loadwallet(const Config &config,

WalletContext &context = EnsureWalletContext(request.context);
const std::string name(request.params[0].get_str());
fs::path path(fs::absolute(name, GetWalletDir()));

if (fs::symlink_status(path).type() == fs::file_not_found) {
throw JSONRPCError(RPC_WALLET_NOT_FOUND,
"Wallet " + name + " not found.");
} else if (fs::is_directory(path)) {
// The given filename is a directory. Check that there's a wallet.dat
// file.
fs::path wallet_dat_file = path / "wallet.dat";
if (fs::symlink_status(wallet_dat_file).type() == fs::file_not_found) {
throw JSONRPCError(RPC_WALLET_NOT_FOUND,
"Directory " + name +
" does not contain a wallet.dat file.");
}
}

DatabaseOptions options;
DatabaseStatus status;
options.require_existing = true;
bilingual_str error;
std::vector<bilingual_str> warnings;
std::optional<bool> load_on_start =
Expand All @@ -3155,7 +3141,13 @@ static UniValue loadwallet(const Config &config,
std::shared_ptr<CWallet> const wallet = LoadWallet(
*context.chain, name, load_on_start, options, status, error, warnings);
if (!wallet) {
throw JSONRPCError(RPC_WALLET_ERROR, error.original);
// Map bad format to not found, since bad format is returned when the
// wallet directory exists, but doesn't contain a data file.
RPCErrorCode code = status == DatabaseStatus::FAILED_NOT_FOUND ||
status == DatabaseStatus::FAILED_BAD_FORMAT
? RPC_WALLET_NOT_FOUND
: RPC_WALLET_ERROR;
throw JSONRPCError(code, error.original);
}

UniValue obj(UniValue::VOBJ);
Expand Down
8 changes: 6 additions & 2 deletions test/functional/rpc_createmultisig.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,12 @@ def do_multisig(self):
try:
node1.loadwallet('wmulti')
except JSONRPCException as e:
if e.error['code'] == -18 and \
'Wallet wmulti not found' in e.error['message']:
path = os.path.join(self.options.tmpdir, "node1", "regtest",
"wallets", "wmulti")
if e.error['code'] == -18 and (
"Wallet file verification failed. Failed to load "
"database path '{}'. Path does not exist.".format(path)
in e.error['message']):
node1.createwallet(wallet_name='wmulti',
disable_private_keys=True)
else:
Expand Down
18 changes: 14 additions & 4 deletions test/functional/wallet_multiwallet.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,13 @@ def wallet_file(name):
assert_equal(set(self.nodes[0].listwallets()), set(wallet_names))

# Fail to load if wallet doesn't exist
assert_raises_rpc_error(-18, 'Wallet wallets not found.',
self.nodes[0].loadwallet, 'wallets')
path = os.path.join(self.options.tmpdir, "node0", "regtest",
"wallets", "wallets")
assert_raises_rpc_error(
-18,
"Wallet file verification failed. Failed to load database path "
"'{}'. Path does not exist.".format(path),
self.nodes[0].loadwallet, 'wallets')

# Fail to load duplicate wallets
path = os.path.join(
Expand Down Expand Up @@ -332,8 +337,13 @@ def wallet_file(name):
# Fail to load if a directory is specified that doesn't contain a
# wallet
os.mkdir(wallet_dir('empty_wallet_dir'))
assert_raises_rpc_error(-18, "Directory empty_wallet_dir does not contain a wallet.dat file",
self.nodes[0].loadwallet, 'empty_wallet_dir')
path = os.path.join(self.options.tmpdir, "node0", "regtest",
"wallets", "empty_wallet_dir")
assert_raises_rpc_error(
-18,
"Wallet file verification failed. Failed to load database "
"path '{}'. Data is not in recognized format.".format(path),
self.nodes[0].loadwallet, 'empty_wallet_dir')

self.log.info("Test dynamic wallet creation.")

Expand Down

0 comments on commit 18aeb32

Please sign in to comment.