rpc, test: Fix error message in unloadwallet

The unloadwallet RPC previously failed with a low-level JSON parsing error
when called without any arguments (wallet_name).

Although this issue was first identified during review of the original unloadwallet
implementation in #13111, it was never addressed.

Raise RPC_INVALID_PARAMETER instead describing that either the RPC endpoint or wallet
name must be provided.

Adding a new functional test for this use case.

Refactor migratewallet to use the same logic as the wallet_name argument handling
is identical.

Co-authored-by:  maflcko <maflcko@users.noreply.github.com>
This commit is contained in:
pablomartin4btc
2025-07-07 23:54:02 -03:00
parent 1fc3a8e8e7
commit 53ac704efd
3 changed files with 8 additions and 25 deletions

View File

@@ -438,8 +438,8 @@ static RPCHelpMan createwallet()
static RPCHelpMan unloadwallet()
{
return RPCHelpMan{"unloadwallet",
"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.",
"Unloads the wallet referenced by the request endpoint or the wallet_name argument.\n"
"If both are specified, they must be identical.",
{
{"wallet_name", RPCArg::Type::STR, RPCArg::DefaultHint{"the wallet name from the RPC endpoint"}, "The name of the wallet to unload. If provided both here and in the RPC endpoint, the two must be identical."},
{"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."},
@@ -456,14 +456,7 @@ static RPCHelpMan unloadwallet()
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
std::string wallet_name;
if (GetWalletNameFromJSONRPCRequest(request, wallet_name)) {
if (!(request.params[0].isNull() || request.params[0].get_str() == wallet_name)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "RPC endpoint wallet and wallet_name parameter specify different wallets");
}
} else {
wallet_name = request.params[0].get_str();
}
const std::string wallet_name{EnsureUniqueWalletName(request, self.MaybeArg<std::string>("wallet_name"))};
WalletContext& context = EnsureWalletContext(request.context);
std::shared_ptr<CWallet> wallet = GetWallet(context, wallet_name);
@@ -685,17 +678,7 @@ static RPCHelpMan migratewallet()
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
std::string wallet_name;
if (GetWalletNameFromJSONRPCRequest(request, wallet_name)) {
if (!(request.params[0].isNull() || request.params[0].get_str() == wallet_name)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "RPC endpoint wallet and wallet_name parameter specify different wallets");
}
} else {
if (request.params[0].isNull()) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Either RPC endpoint wallet or wallet_name parameter must be provided");
}
wallet_name = request.params[0].get_str();
}
const std::string wallet_name{EnsureUniqueWalletName(request, self.MaybeArg<std::string>("wallet_name"))};
SecureString wallet_pass;
wallet_pass.reserve(100);

View File

@@ -518,8 +518,8 @@ class WalletMigrationTest(BitcoinTestFramework):
def test_nonexistent(self):
self.log.info("Check migratewallet errors for nonexistent wallets")
default = self.master_node.get_wallet_rpc(self.default_wallet_name)
assert_raises_rpc_error(-8, "RPC endpoint wallet and wallet_name parameter specify different wallets", default.migratewallet, "someotherwallet")
assert_raises_rpc_error(-8, "Either RPC endpoint wallet or wallet_name parameter must be provided", self.master_node.migratewallet)
assert_raises_rpc_error(-8, "The RPC endpoint wallet and the wallet name parameter specify different wallets", default.migratewallet, "someotherwallet")
assert_raises_rpc_error(-8, "Either the RPC endpoint wallet or the wallet name parameter must be provided", self.master_node.migratewallet)
assert_raises_rpc_error(-4, "Error: Wallet does not exist", self.master_node.migratewallet, "notawallet")
def test_unloaded_by_path(self):

View File

@@ -343,10 +343,10 @@ class MultiWalletTest(BitcoinTestFramework):
self.log.info("Test dynamic wallet unloading")
# Test `unloadwallet` errors
assert_raises_rpc_error(-3, "JSON value of type null is not of expected type string", self.nodes[0].unloadwallet)
assert_raises_rpc_error(-8, "Either the RPC endpoint wallet or the wallet name parameter must be provided", 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, "RPC endpoint wallet and wallet_name parameter specify different wallets", w1.unloadwallet, "w2"),
assert_raises_rpc_error(-8, "The RPC endpoint wallet and the wallet name parameter specify different wallets", w1.unloadwallet, "w2"),
# Successfully unload the specified wallet name
self.nodes[0].unloadwallet("w1")