From e526b3d34c20cd723b08252638d10a7584b697cb Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Mon, 17 Jul 2017 05:42:30 -0400 Subject: [PATCH 1/2] Fix misleading "Method not found" multiwallet errors Raise RPC_WALLET_NOT_SPECIFIED instead of RPC_METHOD_NOT_FOUND when a required wallet filename was not specified in an RPC call. Also raise more specific RPC_WALLET_NOT_FOUND error instead of RPC_INVALID_PARAMETER in case an invalid wallet was specified, for consistency. --- src/bitcoin-cli.cpp | 4 ++++ src/rpc/protocol.h | 2 ++ src/wallet/rpcwallet.cpp | 15 ++++++++------- test/functional/multiwallet.py | 5 ++++- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 445b9d8e8..c1a7c927e 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -337,6 +337,10 @@ int CommandLineRPC(int argc, char *argv[]) if (errMsg.isStr()) strPrint += "error message:\n"+errMsg.get_str(); + + if (errCode.isNum() && errCode.get_int() == RPC_WALLET_NOT_SPECIFIED) { + strPrint += "\nTry adding \"-rpcwallet=\" option to bitcoin-cli command line."; + } } } else { // Result diff --git a/src/rpc/protocol.h b/src/rpc/protocol.h index 70f7092cf..4bd4702d6 100644 --- a/src/rpc/protocol.h +++ b/src/rpc/protocol.h @@ -82,6 +82,8 @@ enum RPCErrorCode RPC_WALLET_WRONG_ENC_STATE = -15, //!< Command given in wrong wallet encryption state (encrypting an encrypted wallet etc.) RPC_WALLET_ENCRYPTION_FAILED = -16, //!< Failed to encrypt the wallet RPC_WALLET_ALREADY_UNLOCKED = -17, //!< Wallet is already unlocked + RPC_WALLET_NOT_FOUND = -18, //!< Invalid wallet specified + RPC_WALLET_NOT_SPECIFIED = -19, //!< No wallet specified (error when there are multiple wallets loaded) }; UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params, const UniValue& id); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index e956adaf8..64dc70f2a 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -43,7 +43,7 @@ CWallet *GetWalletForJSONRPCRequest(const JSONRPCRequest& request) return pwallet; } } - throw JSONRPCError(RPC_INVALID_PARAMETER, "Requested wallet does not exist or is not loaded"); + throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Requested wallet does not exist or is not loaded"); } return ::vpwallets.size() == 1 || (request.fHelp && ::vpwallets.size() > 0) ? ::vpwallets[0] : nullptr; } @@ -57,13 +57,14 @@ std::string HelpRequiringPassphrase(CWallet * const pwallet) bool EnsureWalletIsAvailable(CWallet * const pwallet, bool avoidException) { - if (!pwallet) { - if (!avoidException) - throw JSONRPCError(RPC_METHOD_NOT_FOUND, "Method not found (disabled)"); - else - return false; + if (pwallet) return true; + if (avoidException) return false; + if (::vpwallets.empty()) { + // Wallet RPC methods are disabled if no wallets are loaded. + throw JSONRPCError(RPC_METHOD_NOT_FOUND, "Method not found (disabled)"); } - return true; + throw JSONRPCError(RPC_WALLET_NOT_SPECIFIED, + "Wallet file not specified (must request wallet RPC through /wallet/ uri-path)."); } void EnsureWalletIsUnlocked(CWallet * const pwallet) diff --git a/test/functional/multiwallet.py b/test/functional/multiwallet.py index f0929a8e2..173f06cf5 100755 --- a/test/functional/multiwallet.py +++ b/test/functional/multiwallet.py @@ -21,8 +21,11 @@ class MultiWalletTest(BitcoinTestFramework): w1 = self.nodes[0] / "wallet/w1" w1.generate(1) + # accessing invalid wallet fails + assert_raises_jsonrpc(-18, "Requested wallet does not exist or is not loaded", (self.nodes[0] / "wallet/bad").getwalletinfo) + # accessing wallet RPC without using wallet endpoint fails - assert_raises_jsonrpc(-32601, "Method not found", self.nodes[0].getwalletinfo) + assert_raises_jsonrpc(-19, "Wallet file not specified", self.nodes[0].getwalletinfo) # check w1 wallet balance w1_info = w1.getwalletinfo() From df389bca2048f5814387987463fe3298766bc6b6 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Wed, 26 Jul 2017 09:35:17 -0400 Subject: [PATCH 2/2] Change wallet method disabled error text Not strictly backwards compatible because the error is not new in this release. --- src/wallet/rpcwallet.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 64dc70f2a..14560860c 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -60,8 +60,13 @@ bool EnsureWalletIsAvailable(CWallet * const pwallet, bool avoidException) if (pwallet) return true; if (avoidException) return false; if (::vpwallets.empty()) { - // Wallet RPC methods are disabled if no wallets are loaded. - throw JSONRPCError(RPC_METHOD_NOT_FOUND, "Method not found (disabled)"); + // Note: It isn't currently possible to trigger this error because + // wallet RPC methods aren't registered unless a wallet is loaded. But + // this error is being kept as a precaution, because it's possible in + // the future that wallet RPC methods might get or remain registered + // when no wallets are loaded. + throw JSONRPCError( + RPC_METHOD_NOT_FOUND, "Method not found (wallet method is disabled because no wallet is loaded)"); } throw JSONRPCError(RPC_WALLET_NOT_SPECIFIED, "Wallet file not specified (must request wallet RPC through /wallet/ uri-path).");