Merge #21574: Drop JSONRPCRequest constructors after #21366

9044522ef76f880760165d98fab024802ccfc062 Drop JSONRPCRequest constructors after #21366 (Russell Yanofsky)

Pull request description:

  This just makes an additional simplification after #21366 replaced
  util::Ref with std::any. It was originally suggested
  https://github.com/bitcoin/bitcoin/pull/21366#issuecomment-792044351 but
  delayed for a followup. It would have prevented usage bug
  https://github.com/bitcoin/bitcoin/pull/21572.

ACKs for top commit:
  promag:
    ACK 9044522ef76f880760165d98fab024802ccfc062, fixed conflict in src/wallet/interfaces.cpp.

Tree-SHA512: e909411b8f75013620b94e1a609296befb832fdcb574cd2e6689bfe3c636b03cd4ac1ccb2b32b532daf0f2131bb043464024966310fffc7e3cad77713d4bd0ef

src/bitcoind.cpp

@@ -234,7 +234,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 (...) {
         PrintExceptionContinue(std::current_exception(), "AppInit()");
     }

src/httprpc.cpp

@@ -201,8 +201,8 @@ static bool HTTPReq_JSONRPC(const CoreContext& context, HTTPRequest* req)
         return rpcRequest.send_reply(HTTP_UNAUTHORIZED);
     }
 
-    JSONRPCRequest jreq(context);
-
+    JSONRPCRequest jreq;
+    jreq.context = context;
     jreq.peerAddr = req->GetPeer().ToString();
     if (!RPCAuthorized(authHeader.second, rpcRequest.user)) {
         LogPrintf("ThreadRPCServer incorrect password attempt from %s\n", jreq.peerAddr);
@@ -335,7 +335,7 @@ bool StartHTTPRPC(const CoreContext& 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);

src/init.cpp

@@ -960,7 +960,7 @@ static bool InitSanityCheck()
     return true;
 }
 
-static bool AppInitServers(const CoreContext& context, NodeContext& node)
+static bool AppInitServers(NodeContext& node)
 {
     const ArgsManager& args = *Assert(node.args);
     RPCServer::OnStarted(&OnRPCStarted);
@@ -969,9 +969,9 @@ static bool AppInitServers(const CoreContext& 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;
 }
@@ -1536,7 +1536,7 @@ bool AppInitInterfaces(NodeContext& node)
     return true;
 }
 
-bool AppInitMain(const CoreContext& 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();
@@ -1643,7 +1643,7 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
      */
     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."));
     }
 

src/init.h

@@ -62,7 +62,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 CoreContext& context, NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info = nullptr);
+bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info = nullptr);
 void PrepareShutdown(NodeContext& node);
 
 /**

src/node/interfaces.cpp

@@ -318,7 +318,7 @@ public:
     }
     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
     {
@@ -480,7 +480,8 @@ public:
     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;
@@ -581,15 +582,8 @@ public:
         m_gov.setContext(context);
         m_llmq.setContext(context);
         m_masternodeSync.setContext(context);
-
-        if (context) {
-            m_context_ref = *context;
-        } else {
-            m_context_ref = std::monostate{};
-        }
     }
     NodeContext* m_context{nullptr};
-    CoreContext m_context_ref;
 };
 
 bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<RecursiveMutex>& lock, const CChain& active)

src/rest.cpp

@@ -346,7 +346,8 @@ static bool rest_chaininfo(const CoreContext& context, HTTPRequest* req, const s
 
     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";
@@ -724,7 +725,7 @@ static const struct {
 void StartREST(const CoreContext& 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);
     }
 }

src/rpc/request.cpp

@@ -185,7 +185,7 @@ void JSONRPCRequest::parse(const UniValue& valRequest)
         throw JSONRPCError(RPC_INVALID_REQUEST, "Params must be an array or object");
 }
 
-const JSONRPCRequest JSONRPCRequest::squashed() const
+JSONRPCRequest JSONRPCRequest::squashed() const
 {
    if (params.empty()) {
         return *this;

src/rpc/request.h

@@ -36,22 +36,11 @@ public:
     std::string URI;
     std::string authUser;
     std::string peerAddr;
-    const CoreContext& context;
-
-    explicit JSONRPCRequest(const CoreContext& 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 CoreContext& context)
-        : id(other.id), strMethod(other.strMethod), params(other.params), mode(other.mode), URI(other.URI),
-          authUser(other.authUser), peerAddr(other.peerAddr), context(context)
-    {
-    }
+    CoreContext context;
 
     void parse(const UniValue& valRequest);
     // Returns new JSONRPCRequest with the first param "squashed' into strMethod
-    const JSONRPCRequest squashed() const;
+    JSONRPCRequest squashed() const;
 };
 
 #endif // BITCOIN_RPC_REQUEST_H

src/test/rpc_tests.cpp

@@ -52,7 +52,8 @@ UniValue RPCTestingSetup::TransformParams(const UniValue& params, std::vector<st
     CRPCCommand command{"category", "method", [&](const JSONRPCRequest& request, UniValue&, bool) -> bool { transformed_params = request.params; return true; }, arg_names, /*unique_id=*/0};
     table.appendCommand("method", &command);
     CoreContext context{m_node};
-    JSONRPCRequest request(context);
+    JSONRPCRequest request;
+    request.context = context;
     request.strMethod = "method";
     request.params = params;
     if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished();
@@ -66,7 +67,8 @@ UniValue RPCTestingSetup::CallRPC(std::string args)
     std::string strMethod = vArgs[0];
     vArgs.erase(vArgs.begin());
     CoreContext context{m_node};
-    JSONRPCRequest request(context);
+    JSONRPCRequest request;
+    request.context = context;
     request.strMethod = strMethod;
     request.params = RPCConvertValues(strMethod, vArgs);
     if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished();

src/wallet/interfaces.cpp

@@ -573,7 +573,9 @@ public:
     {
         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()));
         }

src/wallet/test/wallet_tests.cpp

@@ -229,8 +229,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);
-        CoreContext context{m_node};
-        JSONRPCRequest request(context);
+        JSONRPCRequest request;
         request.params.setArray();
         request.params.push_back(keys);
 
@@ -281,9 +280,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
             AddWallet(wallet);
             wallet->SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash());
         }
-        CoreContext context{m_node};
-        JSONRPCRequest request(context);
-
+        JSONRPCRequest request;
         request.params.setArray();
         request.params.push_back(backup_file);
 
@@ -298,8 +295,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
         LOCK(wallet->cs_wallet);
         wallet->SetupLegacyScriptPubKeyMan();
 
-        CoreContext context{m_node};
-        JSONRPCRequest request(context);
+        JSONRPCRequest request;
         request.params.setArray();
         request.params.push_back(backup_file);
         AddWallet(wallet);
@@ -324,7 +320,8 @@ BOOST_FIXTURE_TEST_CASE(rpc_getaddressinfo, TestChain100Setup)
     wallet->SetupLegacyScriptPubKeyMan();
     AddWallet(wallet);
     CoreContext context{m_node};
-    JSONRPCRequest request(context);
+    JSONRPCRequest request;
+    request.context = context;
     UniValue response;
 
     // test p2pkh
@@ -798,7 +795,9 @@ public:
         CoreContext context{m_node};
         std::vector<CRecipient> vecRecipients;
         for (auto entry : vecEntries) {
-            vecRecipients.push_back({GetScriptForDestination(DecodeDestination(getnewaddress().HandleRequest(JSONRPCRequest(context)).get_str())), entry.first, entry.second});
+            JSONRPCRequest request;
+            request.context = context;
+            vecRecipients.push_back({GetScriptForDestination(DecodeDestination(getnewaddress().HandleRequest(request).get_str())), entry.first, entry.second});
         }
         return vecRecipients;
     }