diff options
author | Russell Yanofsky <russ@yanofsky.org> | 2017-07-31 11:46:13 -0400 |
---|---|---|
committer | Russell Yanofsky <russ@yanofsky.org> | 2019-03-05 10:20:00 -0400 |
commit | 4e4d9e9f85eaf9c3bec48559bd4cad3e8a9333ca (patch) | |
tree | 32ed7d798e1feaa8e1490d403f4d83760cddcd04 | |
parent | 91868e6288abf9d133620b585bc6de793a11e0e3 (diff) | |
download | bitcoin-4e4d9e9f85eaf9c3bec48559bd4cad3e8a9333ca.tar.xz |
Remove use of CRPCTable::appendCommand in wallet code
This commit does not change behavior.
-rw-r--r-- | src/interfaces/chain.cpp | 47 | ||||
-rw-r--r-- | src/interfaces/chain.h | 5 | ||||
-rw-r--r-- | src/interfaces/wallet.cpp | 3 | ||||
-rw-r--r-- | src/rpc/server.cpp | 61 | ||||
-rw-r--r-- | src/rpc/server.h | 31 | ||||
-rw-r--r-- | src/test/rpc_tests.cpp | 5 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 4 | ||||
-rw-r--r-- | src/wallet/rpcwallet.h | 9 | ||||
-rw-r--r-- | src/wallet/test/wallet_test_fixture.cpp | 2 | ||||
-rw-r--r-- | src/wallet/test/wallet_test_fixture.h | 1 |
10 files changed, 128 insertions, 40 deletions
diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 8ce718cc50..f04d07d49d 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -15,12 +15,15 @@ #include <primitives/block.h> #include <primitives/transaction.h> #include <protocol.h> +#include <rpc/protocol.h> +#include <rpc/server.h> #include <sync.h> #include <threadsafety.h> #include <timedata.h> #include <txmempool.h> #include <ui_interface.h> #include <uint256.h> +#include <univalue.h> #include <util/system.h> #include <validation.h> #include <validationinterface.h> @@ -212,6 +215,45 @@ public: Chain::Notifications* m_notifications; }; +class RpcHandlerImpl : public Handler +{ +public: + RpcHandlerImpl(const CRPCCommand& command) : m_command(command), m_wrapped_command(&command) + { + m_command.actor = [this](const JSONRPCRequest& request, UniValue& result, bool last_handler) { + if (!m_wrapped_command) return false; + try { + return m_wrapped_command->actor(request, result, last_handler); + } catch (const UniValue& e) { + // If this is not the last handler and a wallet not found + // exception was thrown, return false so the next handler can + // try to handle the request. Otherwise, reraise the exception. + if (!last_handler) { + const UniValue& code = e["code"]; + if (code.isNum() && code.get_int() == RPC_WALLET_NOT_FOUND) { + return false; + } + } + throw; + } + }; + ::tableRPC.appendCommand(m_command.name, &m_command); + } + + void disconnect() override final + { + if (m_wrapped_command) { + m_wrapped_command = nullptr; + ::tableRPC.removeCommand(m_command.name, &m_command); + } + } + + ~RpcHandlerImpl() override { disconnect(); } + + CRPCCommand m_command; + const CRPCCommand* m_wrapped_command; +}; + class ChainImpl : public Chain { public: @@ -310,8 +352,11 @@ public: return MakeUnique<NotificationsHandlerImpl>(*this, notifications); } void waitForNotifications() override { SyncWithValidationInterfaceQueue(); } + std::unique_ptr<Handler> handleRpc(const CRPCCommand& command) override + { + return MakeUnique<RpcHandlerImpl>(command); + } }; - } // namespace std::unique_ptr<Chain> MakeChain() { return MakeUnique<ChainImpl>(); } diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 72862617b3..0936449e22 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -16,6 +16,7 @@ class CBlock; class CFeeRate; +class CRPCCommand; class CScheduler; class CValidationState; class uint256; @@ -243,6 +244,10 @@ public: //! Wait for pending notifications to be handled. virtual void waitForNotifications() = 0; + + //! Register handler for RPC. Command is not copied, so reference + //! needs to remain valid until Handler is disconnected. + virtual std::unique_ptr<Handler> handleRpc(const CRPCCommand& command) = 0; }; //! Interface to let node manage chain clients (wallets, or maybe tools for diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 7abbee0912..97eadac6a9 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -514,7 +514,7 @@ public: : m_chain(chain), m_wallet_filenames(std::move(wallet_filenames)) { } - void registerRpcs() override { return RegisterWalletRPCCommands(::tableRPC); } + void registerRpcs() override { return RegisterWalletRPCCommands(m_chain, m_rpc_handlers); } bool verify() override { return VerifyWallets(m_chain, m_wallet_filenames); } bool load() override { return LoadWallets(m_chain, m_wallet_filenames); } void start(CScheduler& scheduler) override { return StartWallets(scheduler); } @@ -524,6 +524,7 @@ public: Chain& m_chain; std::vector<std::string> m_wallet_filenames; + std::vector<std::unique_ptr<Handler>> m_rpc_handlers; }; } // namespace diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index cd90573da0..e803fabcc6 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -30,6 +30,7 @@ static std::string rpcWarmupStatus GUARDED_BY(cs_rpcWarmup) = "RPC server starte static RPCTimerInterface* timerInterface = nullptr; /* Map of name to timer. */ static std::map<std::string, std::unique_ptr<RPCTimerBase> > deadlineTimers; +static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& request, UniValue& result, bool last_handler); struct RPCCommandExecutionInfo { @@ -173,11 +174,11 @@ std::string CRPCTable::help(const std::string& strCommand, const JSONRPCRequest& { std::string strRet; std::string category; - std::set<rpcfn_type> setDone; + std::set<intptr_t> setDone; std::vector<std::pair<std::string, const CRPCCommand*> > vCommands; for (const auto& entry : mapCommands) - vCommands.push_back(make_pair(entry.second->category + entry.first, entry.second)); + vCommands.push_back(make_pair(entry.second.front()->category + entry.first, entry.second.front())); sort(vCommands.begin(), vCommands.end()); JSONRPCRequest jreq(helpreq); @@ -193,9 +194,9 @@ std::string CRPCTable::help(const std::string& strCommand, const JSONRPCRequest& jreq.strMethod = strMethod; try { - rpcfn_type pfn = pcmd->actor; - if (setDone.insert(pfn).second) - (*pfn)(jreq); + UniValue unused_result; + if (setDone.insert(pcmd->unique_id).second) + pcmd->actor(jreq, unused_result, true /* last_handler */); } catch (const std::exception& e) { @@ -337,32 +338,32 @@ CRPCTable::CRPCTable() const CRPCCommand *pcmd; pcmd = &vRPCCommands[vcidx]; - mapCommands[pcmd->name] = pcmd; + mapCommands[pcmd->name].push_back(pcmd); } } -const CRPCCommand *CRPCTable::operator[](const std::string &name) const -{ - std::map<std::string, const CRPCCommand*>::const_iterator it = mapCommands.find(name); - if (it == mapCommands.end()) - return nullptr; - return (*it).second; -} - bool CRPCTable::appendCommand(const std::string& name, const CRPCCommand* pcmd) { if (IsRPCRunning()) return false; - // don't allow overwriting for now - std::map<std::string, const CRPCCommand*>::const_iterator it = mapCommands.find(name); - if (it != mapCommands.end()) - return false; - - mapCommands[name] = pcmd; + mapCommands[name].push_back(pcmd); return true; } +bool CRPCTable::removeCommand(const std::string& name, const CRPCCommand* pcmd) +{ + auto it = mapCommands.find(name); + if (it != mapCommands.end()) { + auto new_end = std::remove(it->second.begin(), it->second.end(), pcmd); + if (it->second.end() != new_end) { + it->second.erase(new_end, it->second.end()); + return true; + } + } + return false; +} + void StartRPC() { LogPrint(BCLog::RPC, "Starting RPC\n"); @@ -543,18 +544,28 @@ UniValue CRPCTable::execute(const JSONRPCRequest &request) const } // Find method - const CRPCCommand *pcmd = tableRPC[request.strMethod]; - if (!pcmd) - throw JSONRPCError(RPC_METHOD_NOT_FOUND, "Method not found"); + auto it = mapCommands.find(request.strMethod); + if (it != mapCommands.end()) { + UniValue result; + for (const auto& command : it->second) { + if (ExecuteCommand(*command, request, result, &command == &it->second.back())) { + return result; + } + } + } + throw JSONRPCError(RPC_METHOD_NOT_FOUND, "Method not found"); +} +static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& request, UniValue& result, bool last_handler) +{ try { RPCCommandExecution execution(request.strMethod); // Execute, convert arguments to array if necessary if (request.params.isObject()) { - return pcmd->actor(transformNamedArguments(request, pcmd->argNames)); + return command.actor(transformNamedArguments(request, command.argNames), result, last_handler); } else { - return pcmd->actor(request); + return command.actor(request, result, last_handler); } } catch (const std::exception& e) diff --git a/src/rpc/server.h b/src/rpc/server.h index 2d62a76f3c..e2a85887ba 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -131,10 +131,31 @@ typedef UniValue(*rpcfn_type)(const JSONRPCRequest& jsonRequest); class CRPCCommand { public: + //! RPC method handler reading request and assigning result. Should return + //! true if request is fully handled, false if it should be passed on to + //! subsequent handlers. + using Actor = std::function<bool(const JSONRPCRequest& request, UniValue& result, bool last_handler)>; + + //! Constructor taking Actor callback supporting multiple handlers. + CRPCCommand(std::string category, std::string name, Actor actor, std::vector<std::string> args, intptr_t unique_id) + : category(std::move(category)), name(std::move(name)), actor(std::move(actor)), argNames(std::move(args)), + unique_id(unique_id) + { + } + + //! Simplified constructor taking plain rpcfn_type function pointer. + CRPCCommand(const char* category, const char* name, rpcfn_type fn, std::initializer_list<const char*> args) + : CRPCCommand(category, name, + [fn](const JSONRPCRequest& request, UniValue& result, bool) { result = fn(request); return true; }, + {args.begin(), args.end()}, intptr_t(fn)) + { + } + std::string category; std::string name; - rpcfn_type actor; + Actor actor; std::vector<std::string> argNames; + intptr_t unique_id; }; /** @@ -143,10 +164,9 @@ public: class CRPCTable { private: - std::map<std::string, const CRPCCommand*> mapCommands; + std::map<std::string, std::vector<const CRPCCommand*>> mapCommands; public: CRPCTable(); - const CRPCCommand* operator[](const std::string& name) const; std::string help(const std::string& name, const JSONRPCRequest& helpreq) const; /** @@ -169,9 +189,7 @@ public: * * Returns false if RPC server is already running (dump concurrency protection). * - * Commands cannot be overwritten (returns false). - * - * Commands with different method names but the same callback function will + * Commands with different method names but the same unique_id will * be considered aliases, and only the first registered method name will * show up in the help text command listing. Aliased commands do not have * to have the same behavior. Server and client code can distinguish @@ -179,6 +197,7 @@ public: * register different names, types, and numbers of parameters. */ bool appendCommand(const std::string& name, const CRPCCommand* pcmd); + bool removeCommand(const std::string& name, const CRPCCommand* pcmd); }; bool IsDeprecatedRPCEnabled(const std::string& method); diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index ff48398925..9bb2bf551b 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -31,10 +31,9 @@ UniValue CallRPC(std::string args) request.strMethod = strMethod; request.params = RPCConvertValues(strMethod, vArgs); request.fHelp = false; - BOOST_CHECK(tableRPC[strMethod]); - rpcfn_type method = tableRPC[strMethod]->actor; + if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished(); try { - UniValue result = (*method)(request); + UniValue result = tableRPC.execute(request); return result; } catch (const UniValue& objError) { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 31a3209a49..3a197ec0ef 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4156,8 +4156,8 @@ static const CRPCCommand commands[] = }; // clang-format on -void RegisterWalletRPCCommands(CRPCTable &t) +void RegisterWalletRPCCommands(interfaces::Chain& chain, std::vector<std::unique_ptr<interfaces::Handler>>& handlers) { for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++) - t.appendCommand(commands[vcidx].name, &commands[vcidx]); + handlers.emplace_back(chain.handleRpc(commands[vcidx])); } diff --git a/src/wallet/rpcwallet.h b/src/wallet/rpcwallet.h index 58053bde59..7cf607ccc7 100644 --- a/src/wallet/rpcwallet.h +++ b/src/wallet/rpcwallet.h @@ -5,7 +5,9 @@ #ifndef BITCOIN_WALLET_RPCWALLET_H #define BITCOIN_WALLET_RPCWALLET_H +#include <memory> #include <string> +#include <vector> class CRPCTable; class CWallet; @@ -14,7 +16,12 @@ class UniValue; struct PartiallySignedTransaction; class CTransaction; -void RegisterWalletRPCCommands(CRPCTable &t); +namespace interfaces { +class Chain; +class Handler; +} + +void RegisterWalletRPCCommands(interfaces::Chain& chain, std::vector<std::unique_ptr<interfaces::Handler>>& handlers); /** * Figures out what wallet, if any, to use for a JSONRPCRequest. diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 6a9922b3cd..6a051214a9 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -15,5 +15,5 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName): m_wallet.LoadWallet(fFirstRun); m_wallet.m_chain_notifications_handler = m_chain->handleNotifications(m_wallet); - RegisterWalletRPCCommands(tableRPC); + m_chain_client->registerRpcs(); } diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index edc06a8351..dcfbc55f94 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -19,6 +19,7 @@ struct WalletTestingSetup: public TestingSetup { explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain(); + std::unique_ptr<interfaces::ChainClient> m_chain_client = interfaces::MakeWalletClient(*m_chain, {}); CWallet m_wallet; }; |