Drop JSONRPCRequest constructors after #21366

ryanofsky · ryanofsky · commit 9044522ef76f · 2021-04-07T04:53:26.000-04:00
This just makes an additional simplification after #21366 replaced util::Ref with std::any. It was originally suggested bitcoin/bitcoin#21366 (comment) but delayed for a followup. It would have prevented usage bug bitcoin/bitcoin#21572.
diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp
@@ -224,7 +224,7 @@ static bool AppInit(int argc, char* argv[])
             // If locking the data directory failed, exit immediately
             return false;
         }
-        fRet = AppInitInterfaces(node) && AppInitMain(context, node);
+        fRet = AppInitInterfaces(node) && AppInitMain(node);
     }
     catch (const std::exception& e) {
         PrintExceptionContinue(&e, "AppInit()");
diff --git a/src/httprpc.cpp b/src/httprpc.cpp
@@ -159,7 +159,8 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)
         return false;
     }
 
-    JSONRPCRequest jreq(context);
+    JSONRPCRequest jreq;
+    jreq.context = context;
     jreq.peerAddr = req->GetPeer().ToString();
     if (!RPCAuthorized(authHeader.second, jreq.authUser)) {
         LogPrintf("ThreadRPCServer incorrect password attempt from %s\n", jreq.peerAddr);
@@ -294,7 +295,7 @@ bool StartHTTPRPC(const std::any& context)
     if (!InitRPCAuthentication())
         return false;
 
-    auto handle_rpc = [&context](HTTPRequest* req, const std::string&) { return HTTPReq_JSONRPC(context, req); };
+    auto handle_rpc = [context](HTTPRequest* req, const std::string&) { return HTTPReq_JSONRPC(context, req); };
     RegisterHTTPHandler("/", true, handle_rpc);
     if (g_wallet_init_interface.HasWalletSupport()) {
         RegisterHTTPHandler("/wallet/", false, handle_rpc);
diff --git a/src/init.cpp b/src/init.cpp
@@ -788,7 +788,7 @@ static bool InitSanityCheck()
     return true;
 }
 
-static bool AppInitServers(const std::any& context, NodeContext& node)
+static bool AppInitServers(NodeContext& node)
 {
     const ArgsManager& args = *Assert(node.args);
     RPCServer::OnStarted(&OnRPCStarted);
@@ -797,9 +797,9 @@ static bool AppInitServers(const std::any& context, NodeContext& node)
         return false;
     StartRPC();
     node.rpc_interruption_point = RpcInterruptionPoint;
-    if (!StartHTTPRPC(context))
+    if (!StartHTTPRPC(&node))
         return false;
-    if (args.GetBoolArg("-rest", DEFAULT_REST_ENABLE)) StartREST(context);
+    if (args.GetBoolArg("-rest", DEFAULT_REST_ENABLE)) StartREST(&node);
     StartHTTPServer();
     return true;
 }
@@ -1277,7 +1277,7 @@ bool AppInitInterfaces(NodeContext& node)
     return true;
 }
 
-bool AppInitMain(const std::any& context, NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
+bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 {
     const ArgsManager& args = *Assert(node.args);
     const CChainParams& chainparams = Params();
@@ -1382,7 +1382,7 @@ bool AppInitMain(const std::any& context, NodeContext& node, interfaces::BlockAn
      */
     if (args.GetBoolArg("-server", false)) {
         uiInterface.InitMessage_connect(SetRPCWarmupStatus);
-        if (!AppInitServers(context, node))
+        if (!AppInitServers(node))
             return InitError(_("Unable to start HTTP server. See debug log for details."));
     }
 
diff --git a/src/init.h b/src/init.h
@@ -64,7 +64,7 @@ bool AppInitInterfaces(NodeContext& node);
  * @note This should only be done after daemonization. Call Shutdown() if this function fails.
  * @pre Parameters should be parsed and config file should be read, AppInitLockDataDirectory should have been called.
  */
-bool AppInitMain(const std::any& context, NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info = nullptr);
+bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info = nullptr);
 
 /**
  * Register all arguments with the ArgsManager
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
@@ -80,7 +80,7 @@ class NodeImpl : public Node
     }
     bool appInitMain(interfaces::BlockAndHeaderTipInfo* tip_info) override
     {
-        return AppInitMain(m_context_ref, *m_context, tip_info);
+        return AppInitMain(*m_context, tip_info);
     }
     void appShutdown() override
     {
@@ -244,7 +244,8 @@ class NodeImpl : public Node
     CFeeRate getDustRelayFee() override { return ::dustRelayFee; }
     UniValue executeRpc(const std::string& command, const UniValue& params, const std::string& uri) override
     {
-        JSONRPCRequest req(m_context_ref);
+        JSONRPCRequest req;
+        req.context = m_context;
         req.params = params;
         req.strMethod = command;
         req.URI = uri;
@@ -314,14 +315,8 @@ class NodeImpl : public Node
     void setContext(NodeContext* context) override
     {
         m_context = context;
-        if (context) {
-            m_context_ref = context;
-        } else {
-            m_context_ref.reset();
-        }
     }
     NodeContext* m_context{nullptr};
-    std::any m_context_ref;
 };
 
 bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<RecursiveMutex>& lock, const CChain& active)
diff --git a/src/rest.cpp b/src/rest.cpp
@@ -317,7 +317,8 @@ static bool rest_chaininfo(const std::any& context, HTTPRequest* req, const std:
 
     switch (rf) {
     case RetFormat::JSON: {
-        JSONRPCRequest jsonRequest(context);
+        JSONRPCRequest jsonRequest;
+        jsonRequest.context = context;
         jsonRequest.params = UniValue(UniValue::VARR);
         UniValue chainInfoObject = getblockchaininfo().HandleRequest(jsonRequest);
         std::string strJSON = chainInfoObject.write() + "\n";
@@ -687,7 +688,7 @@ static const struct {
 void StartREST(const std::any& context)
 {
     for (const auto& up : uri_prefixes) {
-        auto handler = [&context, up](HTTPRequest* req, const std::string& prefix) { return up.handler(context, req, prefix); };
+        auto handler = [context, up](HTTPRequest* req, const std::string& prefix) { return up.handler(context, req, prefix); };
         RegisterHTTPHandler(up.prefix, false, handler);
     }
 }
diff --git a/src/rpc/request.h b/src/rpc/request.h
@@ -35,18 +35,7 @@ class JSONRPCRequest
     std::string URI;
     std::string authUser;
     std::string peerAddr;
-    const std::any& context;
-
-    explicit JSONRPCRequest(const std::any& context) : id(NullUniValue), params(NullUniValue), context(context) {}
-
-    //! Initializes request information from another request object and the
-    //! given context. The implementation should be updated if any members are
-    //! added or removed above.
-    JSONRPCRequest(const JSONRPCRequest& other, const std::any& context)
-        : id(other.id), strMethod(other.strMethod), params(other.params), mode(other.mode), URI(other.URI),
-          authUser(other.authUser), peerAddr(other.peerAddr), context(context)
-    {
-    }
+    std::any context;
 
     void parse(const UniValue& valRequest);
 };
diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp
@@ -33,8 +33,8 @@ UniValue RPCTestingSetup::CallRPC(std::string args)
     boost::split(vArgs, args, boost::is_any_of(" \t"));
     std::string strMethod = vArgs[0];
     vArgs.erase(vArgs.begin());
-    std::any context{&m_node};
-    JSONRPCRequest request(context);
+    JSONRPCRequest request;
+    request.context = &m_node;
     request.strMethod = strMethod;
     request.params = RPCConvertValues(strMethod, vArgs);
     if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished();
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
@@ -514,15 +514,19 @@ class WalletClientImpl : public WalletClient
     {
         for (const CRPCCommand& command : GetWalletRPCCommands()) {
             m_rpc_commands.emplace_back(command.category, command.name, [this, &command](const JSONRPCRequest& request, UniValue& result, bool last_handler) {
-                return command.actor({request, &m_context}, result, last_handler);
+                JSONRPCRequest wallet_request = request;
+                wallet_request.context = &m_context;
+                return command.actor(wallet_request, result, last_handler);
             }, command.argNames, command.unique_id);
             m_rpc_handlers.emplace_back(m_context.chain->handleRpc(m_rpc_commands.back()));
         }
 
 #ifdef ENABLE_EXTERNAL_SIGNER
         for (const CRPCCommand& command : GetSignerRPCCommands()) {
             m_rpc_commands.emplace_back(command.category, command.name, [this, &command](const JSONRPCRequest& request, UniValue& result, bool last_handler) {
-                return command.actor({request, &m_context}, result, last_handler);
+                JSONRPCRequest wallet_request = request;
+                wallet_request.context = &m_context;
+                return command.actor(wallet_request, result, last_handler);
             }, command.argNames, command.unique_id);
             m_rpc_handlers.emplace_back(m_context.chain->handleRpc(m_rpc_commands.back()));
         }
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
@@ -213,8 +213,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup)
         key.pushKV("timestamp", newTip->GetBlockTimeMax() + TIMESTAMP_WINDOW + 1);
         key.pushKV("internal", UniValue(true));
         keys.push_back(key);
-        std::any context;
-        JSONRPCRequest request(context);
+        JSONRPCRequest request;
         request.params.setArray();
         request.params.push_back(keys);
 
@@ -265,8 +264,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
             AddWallet(wallet);
             wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
         }
-        std::any context;
-        JSONRPCRequest request(context);
+        JSONRPCRequest request;
         request.params.setArray();
         request.params.push_back(backup_file);
 
@@ -281,8 +279,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
         LOCK(wallet->cs_wallet);
         wallet->SetupLegacyScriptPubKeyMan();
 
-        std::any context;
-        JSONRPCRequest request(context);
+        JSONRPCRequest request;
         request.params.setArray();
         request.params.push_back(backup_file);
         AddWallet(wallet);

Original file line number	Diff line number	Diff line change
`@@ -224,7 +224,7 @@ static bool AppInit(int argc, char* argv[])`
`224`	`224`	`// If locking the data directory failed, exit immediately`
`225`	`225`	`return false;`
`226`	`226`	`}`
`227`		`- fRet = AppInitInterfaces(node) && AppInitMain(context, node);`
	`227`	`+ fRet = AppInitInterfaces(node) && AppInitMain(node);`
`228`	`228`	`}`
`229`	`229`	`catch (const std::exception& e) {`
`230`	`230`	`PrintExceptionContinue(&e, "AppInit()");`
Original file line number	Diff line number	Diff line change
`@@ -788,7 +788,7 @@ static bool InitSanityCheck()`
`788`	`788`	`return true;`
`789`	`789`	`}`
`790`	`790`
`791`		`-static bool AppInitServers(const std::any& context, NodeContext& node)`
	`791`	`+static bool AppInitServers(NodeContext& node)`
`792`	`792`	`{`
`793`	`793`	`const ArgsManager& args = *Assert(node.args);`
`794`	`794`	`RPCServer::OnStarted(&OnRPCStarted);`
`@@ -797,9 +797,9 @@ static bool AppInitServers(const std::any& context, NodeContext& node)`
`797`	`797`	`return false;`
`798`	`798`	`StartRPC();`
`799`	`799`	`node.rpc_interruption_point = RpcInterruptionPoint;`
`800`		`- if (!StartHTTPRPC(context))`
	`800`	`+ if (!StartHTTPRPC(&node))`
`801`	`801`	`return false;`
`802`		`- if (args.GetBoolArg("-rest", DEFAULT_REST_ENABLE)) StartREST(context);`
	`802`	`+ if (args.GetBoolArg("-rest", DEFAULT_REST_ENABLE)) StartREST(&node);`
`803`	`803`	`StartHTTPServer();`
`804`	`804`	`return true;`
`805`	`805`	`}`
`@@ -1277,7 +1277,7 @@ bool AppInitInterfaces(NodeContext& node)`
`1277`	`1277`	`return true;`
`1278`	`1278`	`}`
`1279`	`1279`
`1280`		`-bool AppInitMain(const std::any& context, NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)`
	`1280`	`+bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)`
`1281`	`1281`	`{`
`1282`	`1282`	`const ArgsManager& args = *Assert(node.args);`
`1283`	`1283`	`const CChainParams& chainparams = Params();`
`@@ -1382,7 +1382,7 @@ bool AppInitMain(const std::any& context, NodeContext& node, interfaces::BlockAn`
`1382`	`1382`	`*/`
`1383`	`1383`	`if (args.GetBoolArg("-server", false)) {`
`1384`	`1384`	`uiInterface.InitMessage_connect(SetRPCWarmupStatus);`
`1385`		`- if (!AppInitServers(context, node))`
	`1385`	`+ if (!AppInitServers(node))`
`1386`	`1386`	`return InitError(_("Unable to start HTTP server. See debug log for details."));`
`1387`	`1387`	`}`
`1388`	`1388`
Original file line number	Diff line number	Diff line change
`@@ -80,7 +80,7 @@ class NodeImpl : public Node`
`80`	`80`	`}`
`81`	`81`	`bool appInitMain(interfaces::BlockAndHeaderTipInfo* tip_info) override`
`82`	`82`	`{`
`83`		`- return AppInitMain(m_context_ref, *m_context, tip_info);`
	`83`	`+ return AppInitMain(*m_context, tip_info);`
`84`	`84`	`}`
`85`	`85`	`void appShutdown() override`
`86`	`86`	`{`
`@@ -244,7 +244,8 @@ class NodeImpl : public Node`
`244`	`244`	`CFeeRate getDustRelayFee() override { return ::dustRelayFee; }`
`245`	`245`	`UniValue executeRpc(const std::string& command, const UniValue& params, const std::string& uri) override`
`246`	`246`	`{`
`247`		`- JSONRPCRequest req(m_context_ref);`
	`247`	`+ JSONRPCRequest req;`
	`248`	`+ req.context = m_context;`
`248`	`249`	`req.params = params;`
`249`	`250`	`req.strMethod = command;`
`250`	`251`	`req.URI = uri;`
`@@ -314,14 +315,8 @@ class NodeImpl : public Node`
`314`	`315`	`void setContext(NodeContext* context) override`
`315`	`316`	`{`
`316`	`317`	`m_context = context;`
`317`		`- if (context) {`
`318`		`- m_context_ref = context;`
`319`		`- } else {`
`320`		`- m_context_ref.reset();`
`321`		`- }`
`322`	`318`	`}`
`323`	`319`	`NodeContext* m_context{nullptr};`
`324`		`- std::any m_context_ref;`
`325`	`320`	`};`
`326`	`321`
`327`	`322`	`bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<RecursiveMutex>& lock, const CChain& active)`
Original file line number	Diff line number	Diff line change
`@@ -317,7 +317,8 @@ static bool rest_chaininfo(const std::any& context, HTTPRequest* req, const std:`
`317`	`317`
`318`	`318`	`switch (rf) {`
`319`	`319`	`case RetFormat::JSON: {`
`320`		`- JSONRPCRequest jsonRequest(context);`
	`320`	`+ JSONRPCRequest jsonRequest;`
	`321`	`+ jsonRequest.context = context;`
`321`	`322`	`jsonRequest.params = UniValue(UniValue::VARR);`
`322`	`323`	`UniValue chainInfoObject = getblockchaininfo().HandleRequest(jsonRequest);`
`323`	`324`	`std::string strJSON = chainInfoObject.write() + "\n";`
`@@ -687,7 +688,7 @@ static const struct {`
`687`	`688`	`void StartREST(const std::any& context)`
`688`	`689`	`{`
`689`	`690`	`for (const auto& up : uri_prefixes) {`
`690`		`- auto handler = [&context, up](HTTPRequest* req, const std::string& prefix) { return up.handler(context, req, prefix); };`
	`691`	`+ auto handler = [context, up](HTTPRequest* req, const std::string& prefix) { return up.handler(context, req, prefix); };`
`691`	`692`	`RegisterHTTPHandler(up.prefix, false, handler);`
`692`	`693`	`}`
`693`	`694`	`}`