Merge #6828: refactor: drop fMasternodeMode global, create new context for masternode mode-only logic (ActiveContext), move spun-off signers and EHF signals handler

f4ac71c lint: update circular dependencies allowlist, minor cleanup (Kittywhiskers Van Gogh)
ecb924c refactor: move EHF signals handler to `ActiveContext` (Kittywhiskers Van Gogh)
83707a0 refactor: create notification interface for masternode mode (Kittywhiskers Van Gogh)
d52795c refactor: clean up `CCoinJoinServer`, remove redundant checks (Kittywhiskers Van Gogh)
afec9ae refactor: move CoinJoin server to `ActiveContext` (Kittywhiskers Van Gogh)
fcabb3d refactor: move ChainLock signer to `ActiveContext` (Kittywhiskers Van Gogh)
314f070 refactor: move InstantSend signer to new `ActiveContext` (Kittywhiskers Van Gogh)
be84075 refactor: drop `fMasternodeMode` global (Kittywhiskers Van Gogh)
562ad2b wallet: drop collateral outpoints filtering from balance calculation (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * To allow removing `::fMasternodeMode`, we needed to route all pending global usage and one usage, in `wallet/coinjoin.cpp` ([source](https://github.com/dashpay/dash/blob/873dee049265404f91b7012f73d0e7e8f63078c2/src/wallet/coinjoin.cpp#L184)) required additional refactoring. When calculating a tally of all outpoints from an address, if the node is in masternode mode, we skip over outputs with collateral values.

    `SelectCoinsGroupedByAddresses` is used in three places, creating collaterals ([source](https://github.com/dashpay/dash/blob/873dee049265404f91b7012f73d0e7e8f63078c2/src/coinjoin/client.cpp#L1433)) and denoms ([source](https://github.com/dashpay/dash/blob/873dee049265404f91b7012f73d0e7e8f63078c2/src/coinjoin/client.cpp#L1605)) in the CoinJoin client, calculating the anonymizable balance ([source](https://github.com/dashpay/dash/blob/873dee049265404f91b7012f73d0e7e8f63078c2/src/wallet/coinjoin.cpp#L438)) in wallet, and tests.

    As the wallet will have no way of querying if masternode mode is enabled, we need to pull this logic outside. It isn't required in the CoinJoin client as masternodes cannot participate in CoinJoin as a client ([source](https://github.com/dashpay/dash/blob/873dee049265404f91b7012f73d0e7e8f63078c2/src/coinjoin/client.cpp#L684)) and so `fMasternodeMode` will not evaluate `true`, leaving behind `GetAnonymizableBalance()`~~, which now accepts `fSkipMnCollateral` as the caller _can_ query if the node is in masternode mode ([source](https://github.com/dashpay/dash/blob/08db0fd193ecf124de0f8745eca8b24483a0c062/src/qt/overviewpage.cpp#L397)).~~

    * ~~This requires storing the outpoint's value in order to filter it in `GetAnonymizableBalance()`, which required a change in `CompactTallyItem`'s structure ([source](https://github.com/dashpay/dash/blob/08db0fd193ecf124de0f8745eca8b24483a0c062/src/wallet/wallet.h#L135)).~~ This has been reverted as masternodes can no longer hold collaterals in their local wallet, so we can remove the collateral check outright ([comment](#6828 (comment))).

  * The signers spun-off in [dash#6742](#6742) and [dash#6761](#6761) have been moved to a dedicated masternode mode-only context, `ActiveContext`, as part of an effort to pull out networking-heavy and interactive consensus elements out of `LLMQContext`, for  `bitcoin-chainstate`.

    * As the signers plug into existing managers and aren't standalone, they are _not_ public.

    * Alongside the signers, managers that are masternode mode-only have been moved to `ActiveContext`, namely `CCoinJoinServer` ([source](https://github.com/dashpay/dash/blob/873dee049265404f91b7012f73d0e7e8f63078c2/src/coinjoin/server.cpp#L898)) and `llmq::CEHFSignalsHandler` ([source](https://github.com/dashpay/dash/blob/873dee049265404f91b7012f73d0e7e8f63078c2/src/llmq/ehf_signals.cpp#L41-L43)).

  ## 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)_

Top commit has no ACKs.

Tree-SHA512: d193af61592d7206bc3af5d74f4ee4e5b3b8a91d2806d57e8e2d6bed636f33ceeb82f73cdd0557a5239c67ed99e1c98e42d7e6c412d23f5fa076cfd965d804a5

Author: PastaPastaPasta

src/Makefile.am

@@ -262,6 +262,8 @@ BITCOIN_CORE_H = \
   logging.h \
   logging/timer.h \
   mapport.h \
+  masternode/active/context.h \
+  masternode/active/notificationinterface.h \
   masternode/node.h \
   masternode/meta.h \
   masternode/payments.h \
@@ -520,6 +522,8 @@ libbitcoin_node_a_SOURCES = \
   llmq/snapshot.cpp \
   llmq/utils.cpp \
   mapport.cpp \
+  masternode/active/context.cpp \
+  masternode/active/notificationinterface.cpp \
   masternode/node.cpp \
   masternode/meta.cpp \
   masternode/payments.cpp \

src/chainlock/chainlock.cpp

@@ -44,19 +44,15 @@ bool AreChainLocksEnabled(const CSporkManager& sporkman)
 }
 
 CChainLocksHandler::CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSigningManager& _sigman,
-                                       CSigSharesManager& _shareman, CSporkManager& sporkman, CTxMemPool& _mempool,
-                                       const CMasternodeSync& mn_sync, bool is_masternode) :
+                                       CSporkManager& sporkman, CTxMemPool& _mempool, const CMasternodeSync& mn_sync) :
     m_chainstate{chainstate},
     qman{_qman},
     spork_manager{sporkman},
     mempool{_mempool},
     m_mn_sync{mn_sync},
     scheduler{std::make_unique<CScheduler>()},
     scheduler_thread{
-        std::make_unique<std::thread>(std::thread(util::TraceThread, "cl-schdlr", [&] { scheduler->serviceQueue(); }))},
-    m_signer{is_masternode
-                 ? std::make_unique<chainlock::ChainLockSigner>(chainstate, *this, _sigman, _shareman, sporkman, mn_sync)
-                 : nullptr}
+        std::make_unique<std::thread>(std::thread(util::TraceThread, "cl-schdlr", [&] { scheduler->serviceQueue(); }))}
 {
 }
 

src/chainlock/chainlock.h

@@ -33,7 +33,6 @@ namespace llmq {
 class CInstantSendManager;
 class CQuorumManager;
 class CSigningManager;
-class CSigSharesManager;
 enum class VerifyRecSigStatus;
 
 class CChainLocksHandler final : public chainlock::ChainLockSignerParent
@@ -47,7 +46,7 @@ class CChainLocksHandler final : public chainlock::ChainLockSignerParent
     std::unique_ptr<CScheduler> scheduler;
     std::unique_ptr<std::thread> scheduler_thread;
 
-    std::unique_ptr<chainlock::ChainLockSigner> m_signer{nullptr};
+    chainlock::ChainLockSigner* m_signer{nullptr};
 
     mutable Mutex cs;
     std::atomic<bool> tryLockChainTipScheduled{false};
@@ -68,10 +67,17 @@ class CChainLocksHandler final : public chainlock::ChainLockSignerParent
 
 public:
     explicit CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSigningManager& _sigman,
-                                CSigSharesManager& _shareman, CSporkManager& sporkman, CTxMemPool& _mempool,
-                                const CMasternodeSync& mn_sync, bool is_masternode);
+                                CSporkManager& sporkman, CTxMemPool& _mempool, const CMasternodeSync& mn_sync);
     ~CChainLocksHandler();
 
+    void ConnectSigner(gsl::not_null<chainlock::ChainLockSigner*> signer)
+    {
+        // Prohibit double initialization
+        assert(m_signer == nullptr);
+        m_signer = signer;
+    }
+    void DisconnectSigner() { m_signer = nullptr; }
+
     void Start(const llmq::CInstantSendManager& isman);
     void Stop();
 

src/coinjoin/context.cpp

@@ -7,22 +7,19 @@
 #ifdef ENABLE_WALLET
 #include <coinjoin/client.h>
 #endif // ENABLE_WALLET
-#include <coinjoin/server.h>
+#include <coinjoin/coinjoin.h>
 
-CJContext::CJContext(ChainstateManager& chainman, CConnman& connman, CDeterministicMNManager& dmnman,
-                     CMasternodeMetaMan& mn_metaman, CTxMemPool& mempool,
-                     const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync,
-                     const llmq::CInstantSendManager& isman, std::unique_ptr<PeerManager>& peerman, bool relay_txes) :
-    dstxman{std::make_unique<CDSTXManager>()},
+CJContext::CJContext(ChainstateManager& chainman, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman,
+                     CTxMemPool& mempool, const CActiveMasternodeManager* const mn_activeman,
+                     const CMasternodeSync& mn_sync, const llmq::CInstantSendManager& isman, bool relay_txes) :
 #ifdef ENABLE_WALLET
     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, isman, peerman)}
+    dstxman{std::make_unique<CDSTXManager>()}
 {}
 
 CJContext::~CJContext() {}

src/coinjoin/context.h

@@ -12,16 +12,12 @@
 #include <memory>
 
 class CActiveMasternodeManager;
-class CBlockPolicyEstimator;
-class CCoinJoinServer;
-class CConnman;
 class CDeterministicMNManager;
 class CDSTXManager;
 class ChainstateManager;
 class CMasternodeMetaMan;
 class CMasternodeSync;
 class CTxMemPool;
-class PeerManager;
 namespace llmq {
 class CInstantSendManager;
 };
@@ -34,19 +30,17 @@ class CoinJoinWalletManager;
 struct CJContext {
     CJContext() = delete;
     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, const llmq::CInstantSendManager& isman,
-              std::unique_ptr<PeerManager>& peerman, bool relay_txes);
+    CJContext(ChainstateManager& chainman, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman,
+              CTxMemPool& mempool, const CActiveMasternodeManager* const mn_activeman, const CMasternodeSync& mn_sync,
+              const llmq::CInstantSendManager& isman, bool relay_txes);
     ~CJContext();
 
-    const std::unique_ptr<CDSTXManager> dstxman;
 #ifdef ENABLE_WALLET
     // The main object for accessing mixing
     const std::unique_ptr<CoinJoinWalletManager> walletman;
     const std::unique_ptr<CCoinJoinClientQueueManager> queueman;
 #endif // ENABLE_WALLET
-    const std::unique_ptr<CCoinJoinServer> server;
+    const std::unique_ptr<CDSTXManager> dstxman;
 };
 
 #endif // BITCOIN_COINJOIN_CONTEXT_H

src/coinjoin/server.cpp

@@ -25,7 +25,6 @@
 
 MessageProcessingResult CCoinJoinServer::ProcessMessage(CNode& peer, std::string_view msg_type, CDataStream& vRecv)
 {
-    if (!m_mn_activeman) return {};
     if (!m_mn_sync.IsBlockchainSynced()) return {};
 
     if (msg_type == NetMsgType::DSACCEPT) {
@@ -42,7 +41,6 @@ MessageProcessingResult CCoinJoinServer::ProcessMessage(CNode& peer, std::string
 
 void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
 {
-    assert(m_mn_activeman);
     assert(m_mn_metaman.IsValid());
 
     if (IsSessionReady()) {
@@ -58,7 +56,7 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
     LogPrint(BCLog::COINJOIN, "DSACCEPT -- nDenom %d (%s)  txCollateral %s", dsa.nDenom, CoinJoin::DenominationToString(dsa.nDenom), dsa.txCollateral.ToString()); /* Continued */
 
     auto mnList = m_dmnman.GetListAtChainTip();
-    auto dmn = mnList.GetValidMNByCollateral(m_mn_activeman->GetOutPoint());
+    auto dmn = mnList.GetValidMNByCollateral(m_mn_activeman.GetOutPoint());
     if (!dmn) {
         PushStatus(peer, STATUS_REJECTED, ERR_MN_LIST);
         return;
@@ -69,7 +67,7 @@ void CCoinJoinServer::ProcessDSACCEPT(CNode& peer, CDataStream& vRecv)
             TRY_LOCK(cs_vecqueue, lockRecv);
             if (!lockRecv) return;
 
-            auto mnOutpoint = m_mn_activeman->GetOutPoint();
+            auto mnOutpoint = m_mn_activeman.GetOutPoint();
 
             if (ranges::any_of(vecCoinJoinQueue,
                                [&mnOutpoint](const auto& q){return q.masternodeOutpoint == mnOutpoint;})) {
@@ -183,7 +181,7 @@ MessageProcessingResult CCoinJoinServer::ProcessDSQUEUE(NodeId from, CDataStream
         TRY_LOCK(cs_vecqueue, lockRecv);
         if (!lockRecv) return ret;
         vecCoinJoinQueue.push_back(dsq);
-        m_peerman->RelayDSQ(dsq);
+        m_peerman.RelayDSQ(dsq);
     }
     return ret;
 }
@@ -254,8 +252,6 @@ void CCoinJoinServer::SetNull()
 //
 void CCoinJoinServer::CheckPool()
 {
-    if (!m_mn_activeman) return;
-
     if (int entries = GetEntriesCount(); entries != 0) LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckPool -- entries count %lu\n", entries);
 
     // If we have an entry for each collateral, then create final tx
@@ -317,7 +313,6 @@ void CCoinJoinServer::CreateFinalTransaction()
 void CCoinJoinServer::CommitFinalTransaction()
 {
     AssertLockNotHeld(cs_coinjoin);
-    if (!m_mn_activeman) return; // check and relay final tx only on masternode
 
     CTransactionRef finalTransaction = WITH_LOCK(cs_coinjoin, return MakeTransactionRef(finalMutableTransaction));
     uint256 hashTx = finalTransaction->GetHash();
@@ -341,18 +336,16 @@ void CCoinJoinServer::CommitFinalTransaction()
 
     // create and sign masternode dstx transaction
     if (!m_dstxman.GetDSTX(hashTx)) {
-        CCoinJoinBroadcastTx dstxNew(finalTransaction,
-                                    m_mn_activeman->GetOutPoint(),
-                                    m_mn_activeman->GetProTxHash(),
-                                    GetAdjustedTime());
-        dstxNew.Sign(*m_mn_activeman);
+        CCoinJoinBroadcastTx dstxNew(finalTransaction, m_mn_activeman.GetOutPoint(), m_mn_activeman.GetProTxHash(),
+                                     GetAdjustedTime());
+        dstxNew.Sign(m_mn_activeman);
         m_dstxman.AddDSTX(dstxNew);
     }
 
     LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CommitFinalTransaction -- TRANSMITTING DSTX\n");
 
     CInv inv(MSG_DSTX, hashTx);
-    Assert(m_peerman)->RelayInv(inv);
+    m_peerman.RelayInv(inv);
 
     // Tell the clients it was successful
     RelayCompletedTransaction(MSG_SUCCESS);
@@ -380,7 +373,6 @@ void CCoinJoinServer::CommitFinalTransaction()
 void CCoinJoinServer::ChargeFees() const
 {
     AssertLockNotHeld(cs_coinjoin);
-    if (!m_mn_activeman) return;
 
     //we don't need to charge collateral for every offence.
     if (GetRand<int>(/*nMax=*/100) > 33) return;
@@ -448,8 +440,6 @@ void CCoinJoinServer::ChargeFees() const
 */
 void CCoinJoinServer::ChargeRandomFees() const
 {
-    if (!m_mn_activeman) return;
-
     for (const auto& txCollateral : vecSessionCollaterals) {
         if (GetRand<int>(/*nMax=*/100) > 10) return;
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::ChargeRandomFees -- charging random fees, txCollateral=%s", txCollateral->ToString()); /* Continued */
@@ -463,15 +453,13 @@ void CCoinJoinServer::ConsumeCollateral(const CTransactionRef& txref) const
     if (!ATMPIfSaneFee(m_chainman, txref)) {
         LogPrint(BCLog::COINJOIN, "%s -- ATMPIfSaneFee failed\n", __func__);
     } else {
-        Assert(m_peerman)->RelayTransaction(txref->GetHash());
+        m_peerman.RelayTransaction(txref->GetHash());
         LogPrint(BCLog::COINJOIN, "%s -- Collateral was consumed\n", __func__);
     }
 }
 
 bool CCoinJoinServer::HasTimedOut() const
 {
-    if (!m_mn_activeman) return false;
-
     if (nState == POOL_STATE_IDLE) return false;
 
     int nTimeout = (nState == POOL_STATE_SIGNING) ? COINJOIN_SIGNING_TIMEOUT : COINJOIN_QUEUE_TIMEOUT;
@@ -484,8 +472,6 @@ bool CCoinJoinServer::HasTimedOut() const
 //
 void CCoinJoinServer::CheckTimeout()
 {
-    if (!m_mn_activeman) return;
-
     CheckQueue();
 
     // Too early to do anything
@@ -504,19 +490,15 @@ void CCoinJoinServer::CheckTimeout()
 */
 void CCoinJoinServer::CheckForCompleteQueue()
 {
-    if (!m_mn_activeman) return;
-
     if (nState == POOL_STATE_QUEUE && IsSessionReady()) {
         SetState(POOL_STATE_ACCEPTING_ENTRIES);
 
-        CCoinJoinQueue dsq(nSessionDenom,
-                            m_mn_activeman->GetOutPoint(),
-                            m_mn_activeman->GetProTxHash(),
-                            GetAdjustedTime(), true);
+        CCoinJoinQueue dsq(nSessionDenom, m_mn_activeman.GetOutPoint(), m_mn_activeman.GetProTxHash(),
+                           GetAdjustedTime(), true);
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckForCompleteQueue -- queue is ready, signing and relaying (%s) " /* Continued */
                                      "with %d participants\n", dsq.ToString(), vecSessionCollaterals.size());
-        dsq.Sign(*m_mn_activeman);
-        m_peerman->RelayDSQ(dsq);
+        dsq.Sign(m_mn_activeman);
+        m_peerman.RelayDSQ(dsq);
         WITH_LOCK(cs_vecqueue, vecCoinJoinQueue.push_back(dsq));
     }
 }
@@ -572,7 +554,6 @@ bool CCoinJoinServer::IsInputScriptSigValid(const CTxIn& txin) const
 bool CCoinJoinServer::AddEntry(const CCoinJoinEntry& entry, PoolMessage& nMessageIDRet)
 {
     AssertLockNotHeld(cs_coinjoin);
-    if (!m_mn_activeman) return false;
 
     if (size_t(GetEntriesCount()) >= vecSessionCollaterals.size()) {
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: entries is full!\n", __func__);
@@ -682,8 +663,6 @@ bool CCoinJoinServer::IsSignaturesComplete() const
 
 bool CCoinJoinServer::IsAcceptableDSA(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet) const
 {
-    if (!m_mn_activeman) return false;
-
     // is denom even something legit?
     if (!CoinJoin::IsValidDenomination(dsa.nDenom)) {
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- denom not valid!\n", __func__);
@@ -703,7 +682,7 @@ bool CCoinJoinServer::IsAcceptableDSA(const CCoinJoinAccept& dsa, PoolMessage& n
 
 bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet)
 {
-    if (!m_mn_activeman || nSessionID != 0) return false;
+    if (nSessionID != 0) return false;
 
     // new session can only be started in idle mode
     if (nState != POOL_STATE_IDLE) {
@@ -725,13 +704,11 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage&
 
     if (!fUnitTest) {
         //broadcast that I'm accepting entries, only if it's the first entry through
-        CCoinJoinQueue dsq(nSessionDenom,
-                            m_mn_activeman->GetOutPoint(),
-                            m_mn_activeman->GetProTxHash(),
-                            GetAdjustedTime(), false);
+        CCoinJoinQueue dsq(nSessionDenom, m_mn_activeman.GetOutPoint(), m_mn_activeman.GetProTxHash(),
+                           GetAdjustedTime(), false);
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- signing and relaying new queue: %s\n", dsq.ToString());
-        dsq.Sign(*m_mn_activeman);
-        m_peerman->RelayDSQ(dsq);
+        dsq.Sign(m_mn_activeman);
+        m_peerman.RelayDSQ(dsq);
         LOCK(cs_vecqueue);
         vecCoinJoinQueue.push_back(dsq);
     }
@@ -745,7 +722,7 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage&
 
 bool CCoinJoinServer::AddUserToExistingSession(const CCoinJoinAccept& dsa, PoolMessage& nMessageIDRet)
 {
-    if (!m_mn_activeman || nSessionID == 0 || IsSessionReady()) return false;
+    if (nSessionID == 0 || IsSessionReady()) return false;
 
     if (!IsAcceptableDSA(dsa, nMessageIDRet)) {
         return false;
@@ -881,8 +858,6 @@ void CCoinJoinServer::RelayCompletedTransaction(PoolMessage nMessageID)
 
 void CCoinJoinServer::SetState(PoolState nStateNew)
 {
-    if (!m_mn_activeman) return;
-
     if (nStateNew == POOL_STATE_ERROR) {
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::SetState -- Can't set state to ERROR as a Masternode. \n");
         return;
@@ -895,7 +870,6 @@ void CCoinJoinServer::SetState(PoolState nStateNew)
 
 void CCoinJoinServer::DoMaintenance()
 {
-    if (!m_mn_activeman) return; // only run on masternodes
     if (!m_mn_sync.IsBlockchainSynced()) return;
     if (ShutdownRequested()) return;