Merge #6504: refactor: remove llmq::chainLocksHandler, `llmq::quoru…

…m{InstantSend,Snapshot}Manager`, move to `LLMQContext`

bbb4687 refactor: trim some unused headers, group Dash-specific headers together (Kittywhiskers Van Gogh)
b5a8240 refactor: remove `llmq::quorumSnapshotManager` global, move to `LLMQContext` (Kittywhiskers Van Gogh)
7125330 refactor: pass `llmq::CQuorumSnapshotManager` by argument (Kittywhiskers Van Gogh)
dda0cc6 trivial: clean up arguments formatting in `src/llmq/utils.{cpp,h}` (Kittywhiskers Van Gogh)
9996cd6 trivial: ensure `LLMQContext::{Start,Stop,Interrupt}` are consistent (Kittywhiskers Van Gogh)
76a9885 refactor: remove `llmq::quorumInstantSendManager` global, move to `LLMQContext` (Kittywhiskers Van Gogh)
9c989a1 refactor: trim down `llmq::quorumInstantSendManager` use (Kittywhiskers Van Gogh)
a167c0e refactor: add passthrough functions to `CInstantSendManager` (Kittywhiskers Van Gogh)
41c8c8f refactor: remove `llmq::chainLocksHandler` global, move to `LLMQContext` (Kittywhiskers Van Gogh)
772412a refactor: add passthrough functions to `CChainLocksHandler` (Kittywhiskers Van Gogh)
3161546 refactor: use `GetNodeContext` to access `LLMQContext` members in REST (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * `llmq::CChainLocksHandler` and `llmq::CInstantSendManager` had their usage trimmed by moving functions used in validation logic to `CChainstateHelper`, meaning that `CChainstateHelper` now implements logic of its own alongside initializing `CMNPaymentsProcessor` and `CSpecialTxProcessor`.
    * Care has been taken to avoid returning or accepting as arguments, types related to a particular manager (like `CInstantSendLockPtr`) to avoid tying it down to implementation-specific details but are otherwise passthrough.

  ## Breaking Changes

  None expected.

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK bbb4687
  PastaPastaPasta:
    utACK bbb4687
  knst:
    utACK bbb4687

Tree-SHA512: 64e41631d447bfd63cd7277bb450d9a4a2ec1abd78ff03a82c82c16cf6048df2be7e1400363e46812fc6d93a03f4da7a195e9192651d8d0e18e17854f7f0c0f3

src/coinjoin/client.cpp

@@ -166,6 +166,7 @@ void CCoinJoinClientManager::ProcessMessage(CNode& peer, CChainState& active_cha
 CCoinJoinClientSession::CCoinJoinClientSession(const std::shared_ptr<CWallet>& wallet, CoinJoinWalletManager& walletman,
                                                CCoinJoinClientManager& clientman, CDeterministicMNManager& dmnman,
                                                CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync,
+                                               const llmq::CInstantSendManager& isman,
                                                const std::unique_ptr<CCoinJoinClientQueueManager>& queueman,
                                                bool is_masternode) :
     m_wallet(wallet),
@@ -174,6 +175,7 @@ CCoinJoinClientSession::CCoinJoinClientSession(const std::shared_ptr<CWallet>& w
     m_dmnman(dmnman),
     m_mn_metaman(mn_metaman),
     m_mn_sync(mn_sync),
+    m_isman{isman},
     m_queueman(queueman),
     m_is_masternode{is_masternode}
 {}
@@ -589,7 +591,8 @@ bool CCoinJoinClientSession::SignFinalTransaction(CNode& peer, CChainState& acti
 
     // Make sure all inputs/outputs are valid
     PoolMessage nMessageID{MSG_NOERR};
-    if (!IsValidInOuts(active_chainstate, mempool, finalMutableTransaction.vin, finalMutableTransaction.vout, nMessageID, nullptr)) {
+    if (!IsValidInOuts(active_chainstate, m_isman, mempool, finalMutableTransaction.vin, finalMutableTransaction.vout,
+                       nMessageID, nullptr)) {
         WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::%s -- ERROR! IsValidInOuts() failed: %s\n", __func__, CoinJoin::GetMessageByID(nMessageID).translated);
         UnlockCoins();
         keyHolderStorage.ReturnAll();
@@ -953,7 +956,7 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(ChainstateManager& chainman
                 return false;
             }
         } else {
-            if (!CoinJoin::IsCollateralValid(chainman, mempool, CTransaction(txMyCollateral))) {
+            if (!CoinJoin::IsCollateralValid(chainman, m_isman, mempool, CTransaction(txMyCollateral))) {
                 WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::DoAutomaticDenominating -- invalid collateral, recreating...\n");
                 if (!CreateCollateralTransaction(txMyCollateral, strReason)) {
                     WalletCJLogPrint(m_wallet, "CCoinJoinClientSession::DoAutomaticDenominating -- create collateral error: %s\n", strReason);
@@ -1012,7 +1015,7 @@ bool CCoinJoinClientManager::DoAutomaticDenominating(ChainstateManager& chainman
     AssertLockNotHeld(cs_deqsessions);
     LOCK(cs_deqsessions);
     if (int(deqSessions.size()) < CCoinJoinClientOptions::GetSessions()) {
-        deqSessions.emplace_back(m_wallet, m_walletman, *this, m_dmnman, m_mn_metaman, m_mn_sync, m_queueman,
+        deqSessions.emplace_back(m_wallet, m_walletman, *this, m_dmnman, m_mn_metaman, m_mn_sync, m_isman, m_queueman,
                                  m_is_masternode);
     }
     for (auto& session : deqSessions) {
@@ -1915,7 +1918,8 @@ void CoinJoinWalletManager::Add(const std::shared_ptr<CWallet>& wallet)
         LOCK(cs_wallet_manager_map);
         m_wallet_manager_map.try_emplace(wallet->GetName(),
                                          std::make_unique<CCoinJoinClientManager>(wallet, *this, m_dmnman, m_mn_metaman,
-                                                                                  m_mn_sync, m_queueman, m_is_masternode));
+                                                                                  m_mn_sync, m_isman, m_queueman,
+                                                                                  m_is_masternode));
     }
     g_wallet_init_interface.InitCoinJoinSettings(*this);
 }

src/coinjoin/client.h

@@ -77,13 +77,14 @@ class CoinJoinWalletManager {
 
 public:
     CoinJoinWalletManager(ChainstateManager& chainman, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman,
-                          const CTxMemPool& mempool, const CMasternodeSync& mn_sync,
+                          const CTxMemPool& mempool, const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman,
                           const std::unique_ptr<CCoinJoinClientQueueManager>& queueman, bool is_masternode) :
         m_chainman(chainman),
         m_dmnman(dmnman),
         m_mn_metaman(mn_metaman),
         m_mempool(mempool),
         m_mn_sync(mn_sync),
+        m_isman{isman},
         m_queueman(queueman),
         m_is_masternode{is_masternode}
     {}
@@ -125,6 +126,7 @@ class CoinJoinWalletManager {
     CMasternodeMetaMan& m_mn_metaman;
     const CTxMemPool& m_mempool;
     const CMasternodeSync& m_mn_sync;
+    const llmq::CInstantSendManager& m_isman;
     const std::unique_ptr<CCoinJoinClientQueueManager>& m_queueman;
 
     const bool m_is_masternode;
@@ -142,6 +144,7 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession
     CDeterministicMNManager& m_dmnman;
     CMasternodeMetaMan& m_mn_metaman;
     const CMasternodeSync& m_mn_sync;
+    const llmq::CInstantSendManager& m_isman;
     const std::unique_ptr<CCoinJoinClientQueueManager>& m_queueman;
 
     // Track node type
@@ -201,6 +204,7 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession
     explicit CCoinJoinClientSession(const std::shared_ptr<CWallet>& wallet, CoinJoinWalletManager& walletman,
                                     CCoinJoinClientManager& clientman, CDeterministicMNManager& dmnman,
                                     CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync,
+                                    const llmq::CInstantSendManager& isman,
                                     const std::unique_ptr<CCoinJoinClientQueueManager>& queueman, bool is_masternode);
 
     void ProcessMessage(CNode& peer, CChainState& active_chainstate, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv);
@@ -266,6 +270,7 @@ class CCoinJoinClientManager
     CDeterministicMNManager& m_dmnman;
     CMasternodeMetaMan& m_mn_metaman;
     const CMasternodeSync& m_mn_sync;
+    const llmq::CInstantSendManager& m_isman;
     const std::unique_ptr<CCoinJoinClientQueueManager>& m_queueman;
 
     // Track node type
@@ -302,13 +307,14 @@ class CCoinJoinClientManager
 
     explicit CCoinJoinClientManager(const std::shared_ptr<CWallet>& wallet, CoinJoinWalletManager& walletman,
                                     CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman,
-                                    const CMasternodeSync& mn_sync,
+                                    const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman,
                                     const std::unique_ptr<CCoinJoinClientQueueManager>& queueman, bool is_masternode) :
         m_wallet(wallet),
         m_walletman(walletman),
         m_dmnman(dmnman),
         m_mn_metaman(mn_metaman),
         m_mn_sync(mn_sync),
+        m_isman{isman},
         m_queueman(queueman),
         m_is_masternode{is_masternode}
     {

src/coinjoin/coinjoin.cpp

@@ -207,7 +207,10 @@ std::string CCoinJoinBaseSession::GetStateString() const
     }
 }
 
-bool CCoinJoinBaseSession::IsValidInOuts(CChainState& active_chainstate, const CTxMemPool& mempool, const std::vector<CTxIn>& vin, const std::vector<CTxOut>& vout, PoolMessage& nMessageIDRet, bool* fConsumeCollateralRet) const
+bool CCoinJoinBaseSession::IsValidInOuts(CChainState& active_chainstate, const llmq::CInstantSendManager& isman,
+                                         const CTxMemPool& mempool, const std::vector<CTxIn>& vin,
+                                         const std::vector<CTxOut>& vout, PoolMessage& nMessageIDRet,
+                                         bool* fConsumeCollateralRet) const
 {
     std::set<CScript> setScripPubKeys;
     nMessageIDRet = MSG_NOERR;
@@ -268,7 +271,7 @@ bool CCoinJoinBaseSession::IsValidInOuts(CChainState& active_chainstate, const C
 
         Coin coin;
         if (!viewMemPool.GetCoin(txin.prevout, coin) || coin.IsSpent() ||
-            (coin.nHeight == MEMPOOL_HEIGHT && !llmq::quorumInstantSendManager->IsLocked(txin.prevout.hash))) {
+            (coin.nHeight == MEMPOOL_HEIGHT && !isman.IsLocked(txin.prevout.hash))) {
             LogPrint(BCLog::COINJOIN, "CCoinJoinBaseSession::%s -- ERROR: missing, spent or non-locked mempool input! txin=%s\n", __func__, txin.ToString());
             nMessageIDRet = ERR_MISSING_TX;
             return false;
@@ -313,7 +316,8 @@ bool ATMPIfSaneFee(ChainstateManager& chainman, const CTransactionRef& tx, bool
 }
 
 // check to make sure the collateral provided by the client is valid
-bool CoinJoin::IsCollateralValid(ChainstateManager& chainman, const CTxMemPool& mempool, const CTransaction& txCollateral)
+bool CoinJoin::IsCollateralValid(ChainstateManager& chainman, const llmq::CInstantSendManager& isman,
+                                 const CTxMemPool& mempool, const CTransaction& txCollateral)
 {
     if (txCollateral.vout.empty()) return false;
     if (txCollateral.nLockTime != 0) return false;
@@ -334,7 +338,7 @@ bool CoinJoin::IsCollateralValid(ChainstateManager& chainman, const CTxMemPool&
         Coin coin;
         auto mempoolTx = mempool.get(txin.prevout.hash);
         if (mempoolTx != nullptr) {
-            if (mempool.isSpent(txin.prevout) || !llmq::quorumInstantSendManager->IsLocked(txin.prevout.hash)) {
+            if (mempool.isSpent(txin.prevout) || !isman.IsLocked(txin.prevout.hash)) {
                 LogPrint(BCLog::COINJOIN, "CoinJoin::IsCollateralValid -- spent or non-locked mempool input! txin=%s\n", txin.ToString());
                 return false;
             }

src/coinjoin/coinjoin.h

@@ -33,6 +33,7 @@ class TxValidationState;
 
 namespace llmq {
 class CChainLocksHandler;
+class CInstantSendManager;
 } // namespace llmq
 
 extern RecursiveMutex cs_main;
@@ -308,7 +309,9 @@ class CCoinJoinBaseSession
 
     virtual void SetNull() EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin);
 
-    bool IsValidInOuts(CChainState& active_chainstate, const CTxMemPool& mempool, const std::vector<CTxIn>& vin, const std::vector<CTxOut>& vout, PoolMessage& nMessageIDRet, bool* fConsumeCollateralRet) const;
+    bool IsValidInOuts(CChainState& active_chainstate, const llmq::CInstantSendManager& isman,
+                       const CTxMemPool& mempool, const std::vector<CTxIn>& vin, const std::vector<CTxOut>& vout,
+                       PoolMessage& nMessageIDRet, bool* fConsumeCollateralRet) const;
 
 public:
     int nSessionDenom{0}; // Users must submit a denom matching this
@@ -365,7 +368,8 @@ namespace CoinJoin
     constexpr CAmount GetMaxPoolAmount() { return COINJOIN_ENTRY_MAX_SIZE * vecStandardDenominations.front(); }
 
     /// If the collateral is valid given by a client
-    bool IsCollateralValid(ChainstateManager& chainman, const CTxMemPool& mempool, const CTransaction& txCollateral);
+    bool IsCollateralValid(ChainstateManager& chainman, const llmq::CInstantSendManager& isman,
+                           const CTxMemPool& mempool, const CTransaction& txCollateral);
 }
 
 class CDSTXManager

src/coinjoin/context.cpp

@@ -12,17 +12,17 @@
 CJContext::CJContext(ChainstateManager& chainman, CConnman& connman, CDeterministicMNManager& dmnman,
                      CMasternodeMetaMan& mn_metaman, CTxMemPool& mempool,
                      const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync,
-                     std::unique_ptr<PeerManager>& peerman, bool relay_txes) :
+                     const llmq::CInstantSendManager& isman, std::unique_ptr<PeerManager>& peerman, bool relay_txes) :
     dstxman{std::make_unique<CDSTXManager>()},
 #ifdef ENABLE_WALLET
-    walletman{std::make_unique<CoinJoinWalletManager>(chainman, dmnman, mn_metaman, mempool, mn_sync, queueman,
+    walletman{std::make_unique<CoinJoinWalletManager>(chainman, dmnman, mn_metaman, mempool, mn_sync, isman, queueman,
                                                       /*is_masternode=*/mn_activeman != nullptr)},
     queueman{relay_txes ? std::make_unique<CCoinJoinClientQueueManager>(*walletman, dmnman, mn_metaman, mn_sync,
                                                                         /*is_masternode=*/mn_activeman != nullptr)
                         : nullptr},
 #endif // ENABLE_WALLET
     server{std::make_unique<CCoinJoinServer>(chainman, connman, dmnman, *dstxman, mn_metaman, mempool, mn_activeman,
-                                             mn_sync, peerman)}
+                                             mn_sync, isman, peerman)}
 {}
 
 CJContext::~CJContext() {}

src/coinjoin/context.h

@@ -22,6 +22,9 @@ class CMasternodeMetaMan;
 class CMasternodeSync;
 class CTxMemPool;
 class PeerManager;
+namespace llmq {
+class CInstantSendManager;
+};
 
 #ifdef ENABLE_WALLET
 class CCoinJoinClientQueueManager;
@@ -33,7 +36,8 @@ struct CJContext {
     CJContext(const CJContext&) = delete;
     CJContext(ChainstateManager& chainman, CConnman& connman, CDeterministicMNManager& dmnman,
               CMasternodeMetaMan& mn_metaman, CTxMemPool& mempool, const CActiveMasternodeManager* const mn_activeman,
-              const CMasternodeSync& mn_sync, std::unique_ptr<PeerManager>& peerman, bool relay_txes);
+              const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman,
+              std::unique_ptr<PeerManager>& peerman, bool relay_txes);
     ~CJContext();
 
     const std::unique_ptr<CDSTXManager> dstxman;

src/coinjoin/server.cpp

@@ -583,7 +583,7 @@ bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessag
         return false;
     }
 
-    if (!CoinJoin::IsCollateralValid(m_chainman, mempool, *entry.txCollateral)) {
+    if (!CoinJoin::IsCollateralValid(m_chainman, m_isman, mempool, *entry.txCollateral)) {
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: collateral not valid!\n", __func__);
         nMessageIDRet = ERR_INVALID_COLLATERAL;
         return false;
@@ -617,7 +617,8 @@ bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessag
     }
 
     bool fConsumeCollateral{false};
-    if (!IsValidInOuts(m_chainman.ActiveChainstate(), mempool, vin, entry.vecTxOut, nMessageIDRet, &fConsumeCollateral)) {
+    if (!IsValidInOuts(m_chainman.ActiveChainstate(), m_isman, mempool, vin, entry.vecTxOut, nMessageIDRet,
+                       &fConsumeCollateral)) {
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR! IsValidInOuts() failed: %s\n", __func__, CoinJoin::GetMessageByID(nMessageIDRet).translated);
         if (fConsumeCollateral) {
             ConsumeCollateral(entry.txCollateral);
@@ -694,7 +695,7 @@ bool CCoinJoinServer::IsAcceptableDSA(const CCoinJoinAccept& dsa, PoolMessage& n
     }
 
     // check collateral
-    if (!fUnitTest && !CoinJoin::IsCollateralValid(m_chainman, mempool, CTransaction(dsa.txCollateral))) {
+    if (!fUnitTest && !CoinJoin::IsCollateralValid(m_chainman, m_isman, mempool, CTransaction(dsa.txCollateral))) {
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- collateral not valid!\n", __func__);
         nMessageIDRet = ERR_INVALID_COLLATERAL;
         return false;

src/coinjoin/server.h

@@ -36,6 +36,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
     CTxMemPool& mempool;
     const CActiveMasternodeManager* const m_mn_activeman;
     const CMasternodeSync& m_mn_sync;
+    const llmq::CInstantSendManager& m_isman;
     std::unique_ptr<PeerManager>& m_peerman;
 
     // Mixing uses collateral transactions to trust parties entering the pool
@@ -94,7 +95,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
     explicit CCoinJoinServer(ChainstateManager& chainman, CConnman& _connman, CDeterministicMNManager& dmnman,
                              CDSTXManager& dstxman, CMasternodeMetaMan& mn_metaman, CTxMemPool& mempool,
                              const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync,
-                             std::unique_ptr<PeerManager>& peerman) :
+                             const llmq::CInstantSendManager& isman, std::unique_ptr<PeerManager>& peerman) :
         m_chainman(chainman),
         connman(_connman),
         m_dmnman(dmnman),
@@ -103,6 +104,7 @@ class CCoinJoinServer : public CCoinJoinBaseSession, public CCoinJoinBaseManager
         mempool(mempool),
         m_mn_activeman(mn_activeman),
         m_mn_sync(mn_sync),
+        m_isman{isman},
         m_peerman(peerman),
         vecSessionCollaterals(),
         fUnitTest(false)

src/dsnotificationinterface.cpp

@@ -91,7 +91,7 @@ void CDSNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, con
 #endif // ENABLE_WALLET
 
     m_llmq_ctx->isman->UpdatedBlockTip(pindexNew);
-    m_llmq_ctx->clhandler->UpdatedBlockTip();
+    m_llmq_ctx->clhandler->UpdatedBlockTip(*m_llmq_ctx->isman);
 
     m_llmq_ctx->qman->UpdatedBlockTip(pindexNew, m_connman, fInitialDownload);
     m_llmq_ctx->qdkgsman->UpdatedBlockTip(pindexNew, fInitialDownload);

src/evo/cbtx.cpp

@@ -62,8 +62,8 @@ bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidati
 
 // This can only be done after the block has been fully processed, as otherwise we won't have the finished MN list
 bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, CDeterministicMNManager& dmnman,
-                          const llmq::CQuorumBlockProcessor& quorum_block_processor, BlockValidationState& state,
-                          const CCoinsViewCache& view)
+                          llmq::CQuorumSnapshotManager& qsnapman, const llmq::CQuorumBlockProcessor& quorum_block_processor,
+                          BlockValidationState& state, const CCoinsViewCache& view)
 {
     if (block.vtx[0]->nType != TRANSACTION_COINBASE) {
         return true;
@@ -87,7 +87,7 @@ bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, CDeter
         static int64_t nTimeMerkleQuorum = 0;
 
         uint256 calculatedMerkleRoot;
-        if (!CalcCbTxMerkleRootMNList(block, pindex->pprev, calculatedMerkleRoot, dmnman, state, view)) {
+        if (!CalcCbTxMerkleRootMNList(block, pindex->pprev, calculatedMerkleRoot, state, dmnman, qsnapman, view)) {
             // pass the state returned by the function above
             return false;
         }
@@ -117,7 +117,8 @@ bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, CDeter
 }
 
 bool CalcCbTxMerkleRootMNList(const CBlock& block, const CBlockIndex* pindexPrev, uint256& merkleRootRet,
-                              CDeterministicMNManager& dmnman, BlockValidationState& state, const CCoinsViewCache& view)
+                              BlockValidationState& state, CDeterministicMNManager& dmnman,
+                              llmq::CQuorumSnapshotManager& qsnapman, const CCoinsViewCache& view)
 {
     try {
         static std::atomic<int64_t> nTimeDMN = 0;
@@ -127,7 +128,7 @@ bool CalcCbTxMerkleRootMNList(const CBlock& block, const CBlockIndex* pindexPrev
         int64_t nTime1 = GetTimeMicros();
 
         CDeterministicMNList tmpMNList;
-        if (!dmnman.BuildNewListFromBlock(block, pindexPrev, state, view, tmpMNList, false)) {
+        if (!dmnman.BuildNewListFromBlock(block, pindexPrev, state, view, tmpMNList, qsnapman, false)) {
             // pass the state returned by the function above
             return false;
         }