Skip to content

cli: Improve error message on multiwallet cli-side commands #26990

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
achow101 merged 1 commit into from
Sep 20, 2024
Merged

cli: Improve error message on multiwallet cli-side commands #26990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/bitcoin-cli.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -950,7 +950,8 @@ static void ParseError(const UniValue& error, std::string& strPrint, int& nRet)
strPrint += ("error message:\n" + err_msg.get_str());
}
if (err_code.isNum() && err_code.getInt<int>() == RPC_WALLET_NOT_SPECIFIED) {
strPrint += "\nTry adding \"-rpcwallet=<filename>\" option to bitcoin-cli command line.";
strPrint += " Or for the CLI, specify the \"-rpcwallet=<walletname>\" option before the command";
strPrint += " (run \"bitcoin-cli -h\" for help or \"bitcoin-cli listwallets\" to see which wallets are currently loaded).";
}
} else {
strPrint = "error: " + error.write();
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/rpc/util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ std::shared_ptr<CWallet> GetWalletForJSONRPCRequest(const JSONRPCRequest& reques
RPC_WALLET_NOT_FOUND, "No wallet is loaded. Load a wallet using loadwallet or create a new one with createwallet. (Note: A default wallet is no longer automatically created)");
}
throw JSONRPCError(RPC_WALLET_NOT_SPECIFIED,
"Wallet file not specified (must request wallet RPC through /wallet/<filename> uri-path).");
"Multiple wallets are loaded. Please select which wallet to use by requesting the RPC through the /wallet/<walletname> URI path.");
}

void EnsureWalletIsUnlocked(const CWallet& wallet)
Expand Down
11 changes: 10 additions & 1 deletion test/functional/interface_bitcoin_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,12 @@
BLOCKS_VALUE_OF_ZERO = 'error: the first argument (number of blocks to generate, default: 1) must be an integer value greater than zero'
TOO_MANY_ARGS = 'error: too many arguments (maximum 2 for nblocks and maxtries)'
WALLET_NOT_LOADED = 'Requested wallet does not exist or is not loaded'
WALLET_NOT_SPECIFIED = 'Wallet file not specified'
WALLET_NOT_SPECIFIED = (
"Multiple wallets are loaded. Please select which wallet to use by requesting the RPC "
"through the /wallet/<walletname> URI path. Or for the CLI, specify the \"-rpcwallet=<walletname>\" "
"option before the command (run \"bitcoin-cli -h\" for help or \"bitcoin-cli listwallets\" to see "
"which wallets are currently loaded)."
)


def cli_get_info_string_to_dict(cli_get_info_string):
Expand Down Expand Up @@ -331,6 +336,10 @@ def run_test(self):
n4 = 10
blocks = self.nodes[0].getblockcount()

self.log.info('Test -generate -rpcwallet=<filename> raise RPC error')
Copy link
Member

@jonatack jonatack Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit if you retouch: s/raise/raises/ and perhaps explain the difference between the failing filename passed and the wallet name

Suggested change
self.log.info('Test -generate -rpcwallet=<filename> raise RPC error')
self.log.info('Test -generate -rpcwallet=<filename> ("wallet.dat" instead of "wallet") raises RPC error')

Copy link
Member Author

@pablomartin4btc pablomartin4btc Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do this on the follow-up where I'll be adding the validation from the 2nd commit that was dropped from this PR.

wallet2_path = f'-rpcwallet={self.nodes[0].wallets_path / wallets[2] / self.wallet_data_filename}'
assert_raises_rpc_error(-18, WALLET_NOT_LOADED, self.nodes[0].cli(wallet2_path, '-generate').echo)

self.log.info('Test -generate -rpcwallet with no args')
generate = self.nodes[0].cli(rpcwallet2, '-generate').send_cli()
assert_equal(set(generate.keys()), {'address', 'blocks'})
Expand Down
4 changes: 2 additions & 2 deletions test/functional/wallet_multiwallet.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ def wallet_file(name):
assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", wallet_bad.getwalletinfo)

# accessing wallet RPC without using wallet endpoint fails
assert_raises_rpc_error(-19, "Wallet file not specified", node.getwalletinfo)
assert_raises_rpc_error(-19, "Multiple wallets are loaded. Please select which wallet", node.getwalletinfo)

w1, w2, w3, w4, *_ = wallets
self.generatetoaddress(node, nblocks=COINBASE_MATURITY + 1, address=w1.getnewaddress(), sync_fun=self.no_op)
Expand Down Expand Up @@ -275,7 +275,7 @@ def wallet_file(name):
loadwallet_name = node.loadwallet(wallet_names[1])
assert_equal(loadwallet_name['name'], wallet_names[1])
assert_equal(node.listwallets(), wallet_names[0:2])
assert_raises_rpc_error(-19, "Wallet file not specified", node.getwalletinfo)
assert_raises_rpc_error(-19, "Multiple wallets are loaded. Please select which wallet", node.getwalletinfo)
w2 = node.get_wallet_rpc(wallet_names[1])
w2.getwalletinfo()

Expand Down