Merge dashpay#6065: fix: add missing checks for not nullptr in rpc for dash specific code

dc15420 refactor: use EnsureConnman and EnsurePeerman when possible (Konstantin Akimov)
dc01f07 fix: add missing checks for not nullptr in rpc for dash specific code (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  There're some usages of unique pointers in RPC code that are not guarded by non-nullptr checks

  ## What was done?
  Added missing `CHECK_NON_FATAL` and refactored some of them to `EnsureConnman` and `EnsurePeerman`

  ## How Has This Been Tested?
  Run unit/functional tests

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK dc15420

Tree-SHA512: 0483d9208f38648cad55804791415b2facfd165514651463f9b48c44d1963be2888471bb6b1f51015bb9ddd160f272b86f42ee67aea66e640c410175412f5f75

Author: PastaPastaPasta

src/rpc/blockchain.cpp

@@ -1605,6 +1605,7 @@ static RPCHelpMan verifychain()
     const int check_depth{request.params[1].isNull() ? DEFAULT_CHECKBLOCKS : request.params[1].get_int()};
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
+    CHECK_NONFATAL(node.evodb);
 
     ChainstateManager& chainman = EnsureChainman(node);
     LOCK(cs_main);
@@ -2962,7 +2963,8 @@ static RPCHelpMan dumptxoutset()
     FILE* file{fsbridge::fopen(temppath, "wb")};
     CAutoFile afile{file, SER_DISK, CLIENT_VERSION};
     NodeContext& node = EnsureAnyNodeContext(request.context);
-    UniValue result = CreateUTXOSnapshot(node, node.chainman->ActiveChainstate(), afile);
+    const ChainstateManager& chainman = EnsureChainman(node);
+    UniValue result = CreateUTXOSnapshot(node, chainman.ActiveChainstate(), afile);
     fs::rename(temppath, path);
 
     result.pushKV("path", path.string());

src/rpc/coinjoin.cpp

@@ -7,6 +7,7 @@
 #include <coinjoin/context.h>
 #include <coinjoin/server.h>
 #include <rpc/blockchain.h>
+#include <rpc/net.h>
 #include <rpc/server.h>
 #include <rpc/util.h>
 #include <util/strencodings.h>
@@ -69,7 +70,8 @@ static RPCHelpMan coinjoin()
         }
 
         CTxMemPool& mempool = EnsureMemPool(node);
-        bool result = cj_clientman->DoAutomaticDenominating(*node.connman, mempool);
+        CConnman& connman = EnsureConnman(node);
+        bool result = cj_clientman->DoAutomaticDenominating(connman, mempool);
         return "Mixing " + (result ? "started successfully" : ("start failed: " + cj_clientman->GetStatuses().original + ", will retry"));
     }
 

src/rpc/evo.cpp

@@ -1697,6 +1697,7 @@ static UniValue protx(const JSONRPCRequest& request)
     } else if (command == "protxinfo") {
         return protx_info(new_request, dmnman, mn_metaman, chainman);
     } else if (command == "protxdiff") {
+        CHECK_NONFATAL(node.llmq_ctx);
         return protx_diff(new_request, dmnman, chainman, *node.llmq_ctx);
     } else if (command == "protxlistdiff") {
         return protx_listdiff(new_request, dmnman, chainman);

src/rpc/governance.cpp

@@ -16,6 +16,7 @@
 #include <node/context.h>
 #include <net.h>
 #include <rpc/blockchain.h>
+#include <rpc/net.h>
 #include <rpc/server.h>
 #include <rpc/util.h>
 #include <governance/common.h>
@@ -53,6 +54,7 @@ static UniValue gobject_count(const JSONRPCRequest& request)
         gobject_count_help(request);
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
+    CHECK_NONFATAL(node.govman);
     return strMode == "json" ? node.govman->ToJson() : node.govman->ToString();
 }
 
@@ -309,6 +311,8 @@ static UniValue gobject_submit(const JSONRPCRequest& request)
     gobject_submit_help(request);
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
+    CHECK_NONFATAL(node.dmnman);
+    CHECK_NONFATAL(node.govman);
 
     if(!node.mn_sync->IsBlockchainSynced()) {
         throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "Must wait for client to sync with masternode network. Try again in a minute or so.");
@@ -395,11 +399,13 @@ static UniValue gobject_submit(const JSONRPCRequest& request)
 
     LogPrintf("gobject(submit) -- Adding locally created governance object - %s\n", strHash);
 
+    PeerManager& peerman = EnsurePeerman(node);
     if (fMissingConfirmations) {
+        CHECK_NONFATAL(node.mn_sync);
         node.govman->AddPostponedObject(govobj);
-        govobj.Relay(*node.peerman, *node.mn_sync);
+        govobj.Relay(peerman, *node.mn_sync);
     } else {
-        node.govman->AddGovernanceObject(govobj, *node.peerman);
+        node.govman->AddGovernanceObject(govobj, peerman);
     }
 
     return govobj.GetHash().ToString();
@@ -410,6 +416,7 @@ static UniValue VoteWithMasternodes(const JSONRPCRequest& request, const std::ma
                              vote_outcome_enum_t eVoteOutcome)
 {
     const NodeContext& node = EnsureAnyNodeContext(request.context);
+    CHECK_NONFATAL(node.govman);
     {
         LOCK(node.govman->cs);
         const CGovernanceObject *pGovObj = node.govman->FindConstGovernanceObject(hash);
@@ -450,7 +457,9 @@ static UniValue VoteWithMasternodes(const JSONRPCRequest& request, const std::ma
         }
 
         CGovernanceException exception;
-        if (node.govman->ProcessVoteAndRelay(vote, exception, *node.connman, *node.peerman)) {
+        CConnman& connman = EnsureConnman(node);
+        PeerManager& peerman = EnsurePeerman(node);
+        if (node.govman->ProcessVoteAndRelay(vote, exception, connman, peerman)) {
             nSuccessful++;
             statusObj.pushKV("result", "success");
         } else {
@@ -493,6 +502,7 @@ static UniValue gobject_vote_many(const JSONRPCRequest& request)
     if (!wallet) return NullUniValue;
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
+    CHECK_NONFATAL(node.dmnman);
 
     uint256 hash(ParseHashV(request.params[0], "Object hash"));
     std::string strVoteSignal = request.params[1].get_str();
@@ -554,6 +564,7 @@ static UniValue gobject_vote_alias(const JSONRPCRequest& request)
     if (!wallet) return NullUniValue;
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
+    CHECK_NONFATAL(node.dmnman);
 
     uint256 hash(ParseHashV(request.params[0], "Object hash"));
     std::string strVoteSignal = request.params[1].get_str();
@@ -690,6 +701,7 @@ static UniValue gobject_list(const JSONRPCRequest& request)
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     CHECK_NONFATAL(node.dmnman);
+    CHECK_NONFATAL(node.govman);
 
     return ListObjects(*node.govman, node.dmnman->GetListAtChainTip(), strCachedSignal, strType, 0);
 }
@@ -857,6 +869,7 @@ static UniValue gobject_getcurrentvotes(const JSONRPCRequest& request)
 
     // FIND OBJECT USER IS LOOKING FOR
     const NodeContext& node = EnsureAnyNodeContext(request.context);
+    CHECK_NONFATAL(node.govman);
 
     LOCK(node.govman->cs);
     const CGovernanceObject* pGovObj = node.govman->FindConstGovernanceObject(hash);
@@ -995,6 +1008,7 @@ static UniValue voteraw(const JSONRPCRequest& request)
     }
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
+    CHECK_NONFATAL(node.govman);
     GovernanceObject govObjType = WITH_LOCK(node.govman->cs, return [&](){
         AssertLockHeld(node.govman->cs);
         const CGovernanceObject *pGovObj = node.govman->FindConstGovernanceObject(hashGovObj);
@@ -1032,8 +1046,11 @@ static UniValue voteraw(const JSONRPCRequest& request)
         throw JSONRPCError(RPC_INTERNAL_ERROR, "Failure to verify vote.");
     }
 
+    CConnman& connman = EnsureConnman(node);
+    PeerManager& peerman = EnsurePeerman(node);
+
     CGovernanceException exception;
-    if (node.govman->ProcessVoteAndRelay(vote, exception, *node.connman, *node.peerman)) {
+    if (node.govman->ProcessVoteAndRelay(vote, exception, connman, peerman)) {
         return "Voted successfully";
     } else {
         throw JSONRPCError(RPC_INTERNAL_ERROR, "Error voting : " + exception.GetMessage());
@@ -1068,6 +1085,7 @@ static UniValue getgovernanceinfo(const JSONRPCRequest& request)
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureAnyChainman(request.context);
+    CHECK_NONFATAL(node.dmnman);
 
     const auto* pindex = WITH_LOCK(cs_main, return chainman.ActiveChain().Tip());
     int nBlockHeight = pindex->nHeight;

src/rpc/masternode.cpp

@@ -15,6 +15,7 @@
 #include <net.h>
 #include <netbase.h>
 #include <rpc/blockchain.h>
+#include <rpc/net.h>
 #include <rpc/server.h>
 #include <rpc/util.h>
 #include <univalue.h>
@@ -47,8 +48,10 @@ static RPCHelpMan masternode_connect()
         throw JSONRPCError(RPC_INTERNAL_ERROR, strprintf("Incorrect masternode address %s", strAddress));
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
-    node.connman->OpenMasternodeConnection(CAddress(addr, NODE_NETWORK));
-    if (!node.connman->IsConnected(CAddress(addr, NODE_NETWORK), CConnman::AllNodes))
+    CConnman& connman = EnsureConnman(node);
+
+    connman.OpenMasternodeConnection(CAddress(addr, NODE_NETWORK));
+    if (!connman.IsConnected(CAddress(addr, NODE_NETWORK), CConnman::AllNodes))
         throw JSONRPCError(RPC_INTERNAL_ERROR, strprintf("Couldn't connect to masternode %s", strAddress));
 
     return "successfully connected";
@@ -133,6 +136,7 @@ static RPCHelpMan masternode_winner()
     }
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
+    CHECK_NONFATAL(node.dmnman);
     return GetNextMasternodeForPayment(*node.dmnman, 10);
 },
     };
@@ -152,6 +156,7 @@ static RPCHelpMan masternode_current()
     }
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
+    CHECK_NONFATAL(node.dmnman);
     return GetNextMasternodeForPayment(*node.dmnman, 1);
 },
     };
@@ -204,6 +209,7 @@ static RPCHelpMan masternode_status()
         [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
 {
     const NodeContext& node = EnsureAnyNodeContext(request.context);
+    CHECK_NONFATAL(node.dmnman);
     if (!node.mn_activeman) {
         throw JSONRPCError(RPC_INTERNAL_ERROR, "This node does not run an active masternode.");
     }
@@ -302,6 +308,7 @@ static RPCHelpMan masternode_winners()
     int nStartHeight = std::max(nChainTipHeight - nCount, 1);
 
     CHECK_NONFATAL(node.dmnman);
+    CHECK_NONFATAL(node.govman);
 
     const auto tip_mn_list = node.dmnman->GetListAtChainTip();
     for (int h = nStartHeight; h <= nChainTipHeight; h++) {
@@ -384,6 +391,8 @@ static RPCHelpMan masternode_payments()
     // A temporary vector which is used to sort results properly (there is no "reverse" in/for UniValue)
     std::vector<UniValue> vecPayments;
 
+    CHECK_NONFATAL(node.chain_helper);
+    CHECK_NONFATAL(node.dmnman);
     while (vecPayments.size() < uint64_t(std::abs(nCount)) && pindex != nullptr) {
         CBlock block;
         if (!ReadBlockFromDisk(block, pindex, Params().GetConsensus())) {
@@ -407,8 +416,6 @@ static RPCHelpMan masternode_payments()
             nBlockFees += nValueIn - tx->GetValueOut();
         }
 
-        CHECK_NONFATAL(node.chain_helper);
-
         std::vector<CTxOut> voutMasternodePayments, voutDummy;
         CMutableTransaction dummyTx;
         CAmount blockSubsidy = GetBlockSubsidy(pindex, Params().GetConsensus());
@@ -548,6 +555,7 @@ static RPCHelpMan masternodelist_helper(bool is_composite)
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
+    CHECK_NONFATAL(node.dmnman);
 
     UniValue obj(UniValue::VOBJ);
 

src/rpc/mining.cpp

@@ -393,6 +393,8 @@ static RPCHelpMan generateblock()
     block.vtx.insert(block.vtx.end(), txs.begin(), txs.end());
 
     {
+        CHECK_NONFATAL(node.evodb);
+
         LOCK(cs_main);
 
         BlockValidationState state;
@@ -704,6 +706,7 @@ static RPCHelpMan getblocktemplate()
             }
 
             LLMQContext& llmq_ctx = EnsureLLMQContext(node);
+            CHECK_NONFATAL(node.evodb);
 
             CBlockIndex* const pindexPrev = active_chain.Tip();
             // TestBlockValidity only supports blocks built on the current Tip
@@ -733,6 +736,7 @@ static RPCHelpMan getblocktemplate()
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid mode");
 
     const CConnman& connman = EnsureConnman(node);
+    CHECK_NONFATAL(node.sporkman);
     if (connman.GetNodeCount(ConnectionDirection::Both) == 0)
         throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, PACKAGE_NAME " is not connected!");
 

src/rpc/misc.cpp

@@ -108,6 +108,7 @@ static RPCHelpMan mnsync()
     std::string strMode = request.params[0].get_str();
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
+    CHECK_NONFATAL(node.mn_sync);
     auto& mn_sync = *node.mn_sync;
 
     if(strMode == "status") {
@@ -170,6 +171,7 @@ static RPCHelpMan spork()
     // basic mode, show info
     std:: string strCommand = request.params[0].get_str();
     const NodeContext& node = EnsureAnyNodeContext(request.context);
+    CHECK_NONFATAL(node.sporkman);
     if (strCommand == "show") {
         UniValue ret(UniValue::VOBJ);
         for (const auto& sporkDef : sporkDefs) {
@@ -215,6 +217,7 @@ static RPCHelpMan sporkupdate()
 
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     PeerManager& peerman = EnsurePeerman(node);
+    CHECK_NONFATAL(node.sporkman);
 
     // SPORK VALUE
     int64_t nValue = request.params[1].get_int64();

src/rpc/quorums.cpp

@@ -7,6 +7,7 @@
 #include <index/txindex.h>
 #include <node/context.h>
 #include <rpc/blockchain.h>
+#include <rpc/net.h>
 #include <rpc/server.h>
 #include <rpc/util.h>
 #include <validation.h>
@@ -279,6 +280,9 @@ static RPCHelpMan quorum_dkgstatus()
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
     const LLMQContext& llmq_ctx = EnsureLLMQContext(node);
+    const CConnman& connman = EnsureConnman(node);
+    CHECK_NONFATAL(node.dmnman);
+    CHECK_NONFATAL(node.sporkman);
 
     int detailLevel = 0;
     if (!request.params[0].isNull()) {
@@ -322,7 +326,7 @@ static RPCHelpMan quorum_dkgstatus()
                     auto allConnections = llmq::utils::GetQuorumConnections(llmq_params, *node.dmnman, *node.sporkman, pQuorumBaseBlockIndex, proTxHash, false);
                     auto outboundConnections = llmq::utils::GetQuorumConnections(llmq_params, *node.dmnman, *node.sporkman, pQuorumBaseBlockIndex, proTxHash, true);
                     std::map<uint256, CAddress> foundConnections;
-                    node.connman->ForEachNode([&](const CNode* pnode) {
+                    connman.ForEachNode([&](const CNode* pnode) {
                         auto verifiedProRegTxHash = pnode->GetVerifiedProRegTxHash();
                         if (!verifiedProRegTxHash.IsNull() && allConnections.count(verifiedProRegTxHash)) {
                             foundConnections.emplace(verifiedProRegTxHash, pnode->addr);
@@ -381,6 +385,7 @@ static RPCHelpMan quorum_memberof()
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
     const LLMQContext& llmq_ctx = EnsureLLMQContext(node);
+    CHECK_NONFATAL(node.dmnman);
 
     uint256 protxHash(ParseHashV(request.params[0], "proTxHash"));
     int scanQuorumsCount = -1;
@@ -747,6 +752,7 @@ static RPCHelpMan quorum_getdata()
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const ChainstateManager& chainman = EnsureChainman(node);
     const LLMQContext& llmq_ctx = EnsureLLMQContext(node);
+    CConnman& connman = EnsureConnman(node);
 
     NodeId nodeId = ParseInt64V(request.params[0], "nodeId");
     Consensus::LLMQType llmqType = static_cast<Consensus::LLMQType>(ParseInt32V(request.params[1], "llmqType"));
@@ -768,7 +774,7 @@ static RPCHelpMan quorum_getdata()
 
     const CBlockIndex* pQuorumBaseBlockIndex = WITH_LOCK(cs_main, return chainman.m_blockman.LookupBlockIndex(quorumHash));
 
-    return node.connman->ForNode(nodeId, [&](CNode* pNode) {
+    return connman.ForNode(nodeId, [&](CNode* pNode) {
         return llmq_ctx.qman->RequestQuorumData(pNode, llmqType, pQuorumBaseBlockIndex, nDataMask, proTxHash);
     });
 },
@@ -791,6 +797,7 @@ static RPCHelpMan quorum_rotationinfo()
 {
     const NodeContext& node = EnsureAnyNodeContext(request.context);
     const LLMQContext& llmq_ctx = EnsureLLMQContext(node);
+    CHECK_NONFATAL(node.dmnman);
 
     llmq::CGetQuorumRotationInfo cmd;
     llmq::CQuorumRotationInfo quorumRotationInfoRet;

src/rpc/rawtransaction.cpp

@@ -490,6 +490,8 @@ static RPCHelpMan getassetunlockstatuses()
         throw JSONRPCError(RPC_INTERNAL_ERROR, "No blocks in chain");
     }
 
+    CHECK_NONFATAL(node.cpoolman);
+
     std::optional<CCreditPool> poolCL{std::nullopt};
     std::optional<CCreditPool> poolOnTip{std::nullopt};
     std::optional<int> nSpecificCoreHeight{std::nullopt};