From dc42bf52c12e197984b20392bad26aa4303ab72f Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Sat, 14 Apr 2012 23:55:05 -0400 Subject: [PATCH 1/3] Encapsulate RPC command dispatch in an array of CRPCCommand's --- src/bitcoinrpc.cpp | 173 +++++++++++++++++++++++---------------------- 1 file changed, 89 insertions(+), 84 deletions(-) diff --git a/src/bitcoinrpc.cpp b/src/bitcoinrpc.cpp index 3c2d69e5..4dfb61bf 100644 --- a/src/bitcoinrpc.cpp +++ b/src/bitcoinrpc.cpp @@ -38,7 +38,16 @@ using namespace json_spirit; void ThreadRPCServer2(void* parg); typedef Value(*rpcfn_type)(const Array& params, bool fHelp); -extern map mapCallTable; + +class CRPCCommand +{ +public: + string name; + rpcfn_type actor; + bool okSafeMode; +}; + +extern map mapCommands; static std::string strRPCUserColonPass; @@ -182,9 +191,10 @@ Value help(const Array& params, bool fHelp) string strRet; set setDone; - for (map::iterator mi = mapCallTable.begin(); mi != mapCallTable.end(); ++mi) + for (map::iterator mi = mapCommands.begin(); mi != mapCommands.end(); ++mi) { - string strMethod = (*mi).first; + CRPCCommand *pcmd = mi->second; + string strMethod = mi->first; // We already filter duplicates, but these deprecated screw up the sort order if (strMethod == "getamountreceived" || strMethod == "getallreceived" || @@ -196,7 +206,7 @@ Value help(const Array& params, bool fHelp) try { Array params; - rpcfn_type pfn = (*mi).second; + rpcfn_type pfn = pcmd->actor; if (setDone.insert(pfn).second) (*pfn)(params, true); } @@ -2003,85 +2013,76 @@ Value getblock(const Array& params, bool fHelp) // Call Table // -pair pCallTable[] = -{ - make_pair("help", &help), - make_pair("stop", &stop), - make_pair("getblockcount", &getblockcount), - make_pair("getblocknumber", &getblocknumber), - make_pair("getconnectioncount", &getconnectioncount), - make_pair("getdifficulty", &getdifficulty), - make_pair("getgenerate", &getgenerate), - make_pair("setgenerate", &setgenerate), - make_pair("gethashespersec", &gethashespersec), - make_pair("getinfo", &getinfo), - make_pair("getmininginfo", &getmininginfo), - make_pair("getnewaddress", &getnewaddress), - make_pair("getaccountaddress", &getaccountaddress), - make_pair("setaccount", &setaccount), - make_pair("getaccount", &getaccount), - make_pair("getaddressesbyaccount", &getaddressesbyaccount), - make_pair("sendtoaddress", &sendtoaddress), - make_pair("getreceivedbyaddress", &getreceivedbyaddress), - make_pair("getreceivedbyaccount", &getreceivedbyaccount), - make_pair("listreceivedbyaddress", &listreceivedbyaddress), - make_pair("listreceivedbyaccount", &listreceivedbyaccount), - make_pair("backupwallet", &backupwallet), - make_pair("keypoolrefill", &keypoolrefill), - make_pair("walletpassphrase", &walletpassphrase), - make_pair("walletpassphrasechange", &walletpassphrasechange), - make_pair("walletlock", &walletlock), - make_pair("encryptwallet", &encryptwallet), - make_pair("validateaddress", &validateaddress), - make_pair("getbalance", &getbalance), - make_pair("move", &movecmd), - make_pair("sendfrom", &sendfrom), - make_pair("sendmany", &sendmany), - make_pair("addmultisigaddress", &addmultisigaddress), - make_pair("getblock", &getblock), - make_pair("getblockhash", &getblockhash), - make_pair("gettransaction", &gettransaction), - make_pair("listtransactions", &listtransactions), - make_pair("signmessage", &signmessage), - make_pair("verifymessage", &verifymessage), - make_pair("getwork", &getwork), - make_pair("listaccounts", &listaccounts), - make_pair("settxfee", &settxfee), - make_pair("getmemorypool", &getmemorypool), - make_pair("listsinceblock", &listsinceblock), - make_pair("dumpprivkey", &dumpprivkey), - make_pair("importprivkey", &importprivkey) -}; -map mapCallTable(pCallTable, pCallTable + sizeof(pCallTable)/sizeof(pCallTable[0])); - -string pAllowInSafeMode[] = -{ - "help", - "stop", - "getblockcount", - "getblocknumber", // deprecated - "getconnectioncount", - "getdifficulty", - "getgenerate", - "setgenerate", - "gethashespersec", - "getinfo", - "getmininginfo", - "getnewaddress", - "getaccountaddress", - "getaccount", - "getaddressesbyaccount", - "backupwallet", - "keypoolrefill", - "walletpassphrase", - "walletlock", - "validateaddress", - "getwork", - "getmemorypool", + +static CRPCCommand vRPCCommands[] = +{ // name function safe mode? + // ------------------------ ----------------------- ---------- + { "help", &help, true }, + { "stop", &stop, true }, + { "getblockcount", &getblockcount, true }, + { "getblocknumber", &getblocknumber, true }, + { "getconnectioncount", &getconnectioncount, true }, + { "getdifficulty", &getdifficulty, true }, + { "getgenerate", &getgenerate, true }, + { "setgenerate", &setgenerate, true }, + { "gethashespersec", &gethashespersec, true }, + { "getinfo", &getinfo, true }, + { "getmininginfo", &getmininginfo, true }, + { "getnewaddress", &getnewaddress, true }, + { "getaccountaddress", &getaccountaddress, true }, + { "setaccount", &setaccount, true }, + { "getaccount", &getaccount, false }, + { "getaddressesbyaccount", &getaddressesbyaccount, true }, + { "sendtoaddress", &sendtoaddress, false }, + { "getreceivedbyaddress", &getreceivedbyaddress, false }, + { "getreceivedbyaccount", &getreceivedbyaccount, false }, + { "listreceivedbyaddress", &listreceivedbyaddress, false }, + { "listreceivedbyaccount", &listreceivedbyaccount, false }, + { "backupwallet", &backupwallet, true }, + { "keypoolrefill", &keypoolrefill, true }, + { "walletpassphrase", &walletpassphrase, true }, + { "walletpassphrasechange", &walletpassphrasechange, false }, + { "walletlock", &walletlock, true }, + { "encryptwallet", &encryptwallet, false }, + { "validateaddress", &validateaddress, true }, + { "getbalance", &getbalance, false }, + { "move", &movecmd, false }, + { "sendfrom", &sendfrom, false }, + { "sendmany", &sendmany, false }, + { "addmultisigaddress", &addmultisigaddress, false }, + { "getblock", &getblock, false }, + { "getblockhash", &getblockhash, false }, + { "gettransaction", &gettransaction, false }, + { "listtransactions", &listtransactions, false }, + { "signmessage", &signmessage, false }, + { "verifymessage", &verifymessage, false }, + { "getwork", &getwork, true }, + { "listaccounts", &listaccounts, false }, + { "settxfee", &settxfee, false }, + { "getmemorypool", &getmemorypool, true }, + { "listsinceblock", &listsinceblock, false }, + { "dumpprivkey", &dumpprivkey, false }, + { "importprivkey", &importprivkey, false }, }; -set setAllowInSafeMode(pAllowInSafeMode, pAllowInSafeMode + sizeof(pAllowInSafeMode)/sizeof(pAllowInSafeMode[0])); +map mapCommands; + +static void RegisterRPCCommands() +{ + static bool registered = false; + if (registered) + return; + registered = true; + + unsigned int vcidx; + for (vcidx = 0; vcidx < (sizeof(vRPCCommands) / sizeof(vRPCCommands[0])); vcidx++) + { + CRPCCommand *pcmd; + pcmd = &vRPCCommands[vcidx]; + mapCommands[pcmd->name] = pcmd; + } +} // @@ -2362,6 +2363,8 @@ void ThreadRPCServer2(void* parg) { printf("ThreadRPCServer started\n"); + RegisterRPCCommands(); + strRPCUserColonPass = mapArgs["-rpcuser"] + ":" + mapArgs["-rpcpassword"]; if (mapArgs["-rpcpassword"] == "") { @@ -2513,13 +2516,15 @@ void ThreadRPCServer2(void* parg) throw JSONRPCError(-32600, "Params must be an array"); // Find method - map::iterator mi = mapCallTable.find(strMethod); - if (mi == mapCallTable.end()) + if (!mapCommands.count(strMethod)) throw JSONRPCError(-32601, "Method not found"); + CRPCCommand *pcmd = mapCommands[strMethod]; + // Observe safe mode string strWarning = GetWarnings("rpc"); - if (strWarning != "" && !GetBoolArg("-disablesafemode") && !setAllowInSafeMode.count(strMethod)) + if (strWarning != "" && !GetBoolArg("-disablesafemode") && + !pcmd->okSafeMode) throw JSONRPCError(-2, string("Safe mode: ") + strWarning); try @@ -2528,7 +2533,7 @@ void ThreadRPCServer2(void* parg) Value result; { LOCK2(cs_main, pwalletMain->cs_wallet); - result = (*(*mi).second)(params, false); + result = pcmd->actor(params, false); } // Send reply From 9862229d4d852279b937c18cdbe076418585844e Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 18 Apr 2012 22:42:17 +0200 Subject: [PATCH 2/3] Encapsulate mapCommands in class CRPCTable --- src/bitcoinrpc.cpp | 65 +++++++++++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/src/bitcoinrpc.cpp b/src/bitcoinrpc.cpp index 4dfb61bf..24b12207 100644 --- a/src/bitcoinrpc.cpp +++ b/src/bitcoinrpc.cpp @@ -47,7 +47,17 @@ public: bool okSafeMode; }; -extern map mapCommands; +class CRPCTable +{ +private: + map mapCommands; +public: + CRPCTable(); + const CRPCCommand* operator[](string name) const; + string help(string name) const; +}; + +const CRPCTable tableRPC; static std::string strRPCUserColonPass; @@ -177,23 +187,13 @@ Object blockToJSON(const CBlock& block, const CBlockIndex* blockindex) /// Note: This interface may still be subject to change. /// - -Value help(const Array& params, bool fHelp) +string CRPCTable::help(string strCommand) const { - if (fHelp || params.size() > 1) - throw runtime_error( - "help [command]\n" - "List commands, or get help for a command."); - - string strCommand; - if (params.size() > 0) - strCommand = params[0].get_str(); - string strRet; set setDone; - for (map::iterator mi = mapCommands.begin(); mi != mapCommands.end(); ++mi) + for (map::const_iterator mi = mapCommands.begin(); mi != mapCommands.end(); ++mi) { - CRPCCommand *pcmd = mi->second; + const CRPCCommand *pcmd = mi->second; string strMethod = mi->first; // We already filter duplicates, but these deprecated screw up the sort order if (strMethod == "getamountreceived" || @@ -226,6 +226,20 @@ Value help(const Array& params, bool fHelp) return strRet; } +Value help(const Array& params, bool fHelp) +{ + if (fHelp || params.size() > 1) + throw runtime_error( + "help [command]\n" + "List commands, or get help for a command."); + + string strCommand; + if (params.size() > 0) + strCommand = params[0].get_str(); + + return tableRPC.help(strCommand); +} + Value stop(const Array& params, bool fHelp) { @@ -2065,15 +2079,8 @@ static CRPCCommand vRPCCommands[] = { "importprivkey", &importprivkey, false }, }; -map mapCommands; - -static void RegisterRPCCommands() +CRPCTable::CRPCTable() { - static bool registered = false; - if (registered) - return; - registered = true; - unsigned int vcidx; for (vcidx = 0; vcidx < (sizeof(vRPCCommands) / sizeof(vRPCCommands[0])); vcidx++) { @@ -2084,6 +2091,13 @@ static void RegisterRPCCommands() } } +const CRPCCommand *CRPCTable::operator[](string name) const +{ + map::const_iterator it = mapCommands.find(name); + if (it == mapCommands.end()) + return NULL; + return (*it).second; +} // // HTTP protocol @@ -2363,8 +2377,6 @@ void ThreadRPCServer2(void* parg) { printf("ThreadRPCServer started\n"); - RegisterRPCCommands(); - strRPCUserColonPass = mapArgs["-rpcuser"] + ":" + mapArgs["-rpcpassword"]; if (mapArgs["-rpcpassword"] == "") { @@ -2516,11 +2528,10 @@ void ThreadRPCServer2(void* parg) throw JSONRPCError(-32600, "Params must be an array"); // Find method - if (!mapCommands.count(strMethod)) + const CRPCCommand *pcmd = tableRPC[strMethod]; + if (!pcmd) throw JSONRPCError(-32601, "Method not found"); - CRPCCommand *pcmd = mapCommands[strMethod]; - // Observe safe mode string strWarning = GetWarnings("rpc"); if (strWarning != "" && !GetBoolArg("-disablesafemode") && From e46704dd904192d8731eae1226805252e5252a0e Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 21 Apr 2012 01:37:34 +0200 Subject: [PATCH 3/3] Expose CRPCTable via bitcoinrpc.h for testing --- src/bitcoinrpc.cpp | 31 +++++-------------------------- src/bitcoinrpc.h | 34 ++++++++++++++++++++++++++++++++++ src/test/rpc_tests.cpp | 9 ++------- 3 files changed, 41 insertions(+), 33 deletions(-) diff --git a/src/bitcoinrpc.cpp b/src/bitcoinrpc.cpp index 24b12207..206347fa 100644 --- a/src/bitcoinrpc.cpp +++ b/src/bitcoinrpc.cpp @@ -10,6 +10,7 @@ #include "net.h" #include "init.h" #include "ui_interface.h" +#include "bitcoinrpc.h" #undef printf #include @@ -22,9 +23,6 @@ #include typedef boost::asio::ssl::stream SSLStream; -#include "json/json_spirit_reader_template.h" -#include "json/json_spirit_writer_template.h" -#include "json/json_spirit_utils.h" #define printf OutputDebugStringF // MinGW 3.4.5 gets "fatal error: had to relocate PCH" if the json headers are // precompiled in headers.h. The problem might be when the pch file goes over @@ -37,27 +35,6 @@ using namespace boost::asio; using namespace json_spirit; void ThreadRPCServer2(void* parg); -typedef Value(*rpcfn_type)(const Array& params, bool fHelp); - -class CRPCCommand -{ -public: - string name; - rpcfn_type actor; - bool okSafeMode; -}; - -class CRPCTable -{ -private: - map mapCommands; -public: - CRPCTable(); - const CRPCCommand* operator[](string name) const; - string help(string name) const; -}; - -const CRPCTable tableRPC; static std::string strRPCUserColonPass; @@ -2028,7 +2005,7 @@ Value getblock(const Array& params, bool fHelp) // -static CRPCCommand vRPCCommands[] = +static const CRPCCommand vRPCCommands[] = { // name function safe mode? // ------------------------ ----------------------- ---------- { "help", &help, true }, @@ -2084,7 +2061,7 @@ CRPCTable::CRPCTable() unsigned int vcidx; for (vcidx = 0; vcidx < (sizeof(vRPCCommands) / sizeof(vRPCCommands[0])); vcidx++) { - CRPCCommand *pcmd; + const CRPCCommand *pcmd; pcmd = &vRPCCommands[vcidx]; mapCommands[pcmd->name] = pcmd; @@ -2785,3 +2762,5 @@ int main(int argc, char *argv[]) return 0; } #endif + +const CRPCTable tableRPC; diff --git a/src/bitcoinrpc.h b/src/bitcoinrpc.h index a9cf3296..6b7293ed 100644 --- a/src/bitcoinrpc.h +++ b/src/bitcoinrpc.h @@ -3,5 +3,39 @@ // Distributed under the MIT/X11 software license, see the accompanying // file license.txt or http://www.opensource.org/licenses/mit-license.php. +#ifndef _BITCOINRPC_H_ +#define _BITCOINRPC_H_ 1 + +#include +#include + +#include "json/json_spirit_reader_template.h" +#include "json/json_spirit_writer_template.h" +#include "json/json_spirit_utils.h" + void ThreadRPCServer(void* parg); int CommandLineRPC(int argc, char *argv[]); + +typedef json_spirit::Value(*rpcfn_type)(const json_spirit::Array& params, bool fHelp); + +class CRPCCommand +{ +public: + std::string name; + rpcfn_type actor; + bool okSafeMode; +}; + +class CRPCTable +{ +private: + std::map mapCommands; +public: + CRPCTable(); + const CRPCCommand* operator[](std::string name) const; + std::string help(std::string name) const; +}; + +extern const CRPCTable tableRPC; + +#endif diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 87462f76..7a438e5d 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -3,16 +3,11 @@ #include "base58.h" #include "util.h" -#include "json/json_spirit_reader_template.h" -#include "json/json_spirit_writer_template.h" -#include "json/json_spirit_utils.h" +#include "bitcoinrpc.h" using namespace std; using namespace json_spirit; -typedef Value(*rpcfn_type)(const Array& params, bool fHelp); -extern map mapCallTable; - BOOST_AUTO_TEST_SUITE(rpc_tests) static Array @@ -36,7 +31,7 @@ struct TestNetFixture BOOST_FIXTURE_TEST_CASE(rpc_addmultisig, TestNetFixture) { - rpcfn_type addmultisig = mapCallTable["addmultisigaddress"]; + rpcfn_type addmultisig = tableRPC["addmultisigaddress"]->actor; // old, 65-byte-long: const char* address1Hex = "0434e3e09f49ea168c5bbf53f877ff4206923858aab7c7e1df25bc263978107c95e35065a27ef6f1b27222db0ec97e0e895eaca603d3ee0d4c060ce3d8a00286c8";