Merge #6841: refactor: move CSigSharesManager to ActiveContext, drop CActiveMasternodeManager from CSigSharesManager

af9a15f lint: apply review suggestions (Kittywhiskers Van Gogh)
6c89f0e refactor: move `RelayRecoveredSig()` call out of `CSigSharesManager` (Kittywhiskers Van Gogh)
0052fca refactor: move `AsyncSignIfMember()` to `CSigSharesManager` (Kittywhiskers Van Gogh)
489cf6e refactor: adjust `CSigSharesManager` ctor arguments (Kittywhiskers Van Gogh)
f6b02b7 refactor: move `CDKGSessionManager` thread management to `ActiveContext` (Kittywhiskers Van Gogh)
74afa28 refactor: move `CSigSharesManager` to `ActiveContext` (Kittywhiskers Van Gogh)
6563ade refactor: remove unused `CSigningManager` from `CChainLocksHandler` ctor (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Depends on #6828

  * Depends on #6839

  * This pull request does _not_ address `CQuorumManager` or `CDKGSessionManager` as both complicate signer separation as they accommodate watch-only mode ([source](https://github.com/dashpay/dash/blob/e5ce1f0bef825d63e81a77dfbe65804da5ae04de/src/llmq/dkgsessionmgr.cpp#L46-L49), [source](https://github.com/dashpay/dash/blob/e5ce1f0bef825d63e81a77dfbe65804da5ae04de/src/llmq/quorums.cpp#L804-L806)) and allow for LLMQ data recovery enablement ([source](https://github.com/dashpay/dash/blob/e5ce1f0bef825d63e81a77dfbe65804da5ae04de/src/llmq/quorums.cpp#L251-L253)), which makes them better suited for a separate PR.

    In contrast, `CSigSharesManager` can be more-or-less wholesale moved to `ActiveContext`, the objective of this PR.

  * As part of isolating masternode-mode logic `CSigningManager::AsyncSignIfMember()`, which accepts `CSigSharesManager` as an argument, was moved to `CSigSharesManager` accepting `CSigningManager` as argument. This required exposing the recovered signatures DB to `CSigningManager`, which has been done with `GetDb()`.

    * As a result of this change and the move to `ActiveContext`, the `quorum sign` RPC will only work if masternode-mode is enabled

  * To allow dropping `CActiveMasternodeManager` from `CSigningManager`, we needed to drop a `RelayRecoveredSig()` call that is only done in masternode-mode ([source](https://github.com/dashpay/dash/blob/e5ce1f0bef825d63e81a77dfbe65804da5ae04de/src/llmq/signing.cpp#L619)). To do that, we rely on the `NotifyRecoveredSig()` signal that is invoked at the tail of `CSigningManager::ProcessRecoveredSig()` ([source](https://github.com/dashpay/dash/blob/e5ce1f0bef825d63e81a77dfbe65804da5ae04de/src/llmq/signing.cpp#L619)) by having `ActiveNotificationInterface` slot in, acting in _response_ to the notification rather than before (i.e. existing behavior).

  ## Breaking Changes

  The `quorum sign` RPC will only work if masternode-mode is enabled.

  ## 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 af9a15f

Tree-SHA512: c769a88b5d311f7562d2fcb9f2953746de6edcb13227034ed2db50c059d31838b925b13de00d4d91bd4fa3ea5d98c13ac3ddfba4cf0c4b17187d7004f7446b1b

Author: PastaPastaPasta

src/chainlock/chainlock.cpp

@@ -43,8 +43,8 @@ bool AreChainLocksEnabled(const CSporkManager& sporkman)
     return sporkman.IsSporkActive(SPORK_19_CHAINLOCKS_ENABLED);
 }
 
-CChainLocksHandler::CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSigningManager& _sigman,
-                                       CSporkManager& sporkman, CTxMemPool& _mempool, const CMasternodeSync& mn_sync) :
+CChainLocksHandler::CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSporkManager& sporkman,
+                                       CTxMemPool& _mempool, const CMasternodeSync& mn_sync) :
     m_chainstate{chainstate},
     qman{_qman},
     spork_manager{sporkman},

src/chainlock/chainlock.h

@@ -18,7 +18,11 @@
 #include <gsl/pointers.h>
 
 #include <atomic>
+#include <cassert>
+#include <chrono>
 #include <map>
+#include <memory>
+#include <thread>
 #include <unordered_map>
 
 class CBlock;
@@ -66,8 +70,8 @@ class CChainLocksHandler final : public chainlock::ChainLockSignerParent
     std::atomic<std::chrono::seconds> lastCleanupTime{0s};
 
 public:
-    explicit CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSigningManager& _sigman,
-                                CSporkManager& sporkman, CTxMemPool& _mempool, const CMasternodeSync& mn_sync);
+    explicit CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSporkManager& sporkman,
+                                CTxMemPool& _mempool, const CMasternodeSync& mn_sync);
     ~CChainLocksHandler();
 
     void ConnectSigner(gsl::not_null<chainlock::ChainLockSigner*> signer)

src/chainlock/signing.cpp

@@ -9,6 +9,7 @@
 
 #include <chainlock/clsig.h>
 #include <instantsend/instantsend.h>
+#include <llmq/signing_shares.h>
 #include <masternode/sync.h>
 #include <spork.h>
 
@@ -140,7 +141,7 @@ void ChainLockSigner::TrySignChainTip(const llmq::CInstantSendManager& isman)
         lastSignedMsgHash = msgHash;
     }
 
-    m_sigman.AsyncSignIfMember(Params().GetConsensus().llmqTypeChainLocks, m_shareman, requestId, msgHash);
+    m_shareman.AsyncSignIfMember(Params().GetConsensus().llmqTypeChainLocks, m_sigman, requestId, msgHash);
 }
 
 void ChainLockSigner::EraseFromBlockHashTxidMap(const uint256& hash)

src/init.cpp

@@ -231,6 +231,9 @@ void Interrupt(NodeContext& node)
     InterruptRPC();
     InterruptREST();
     InterruptTorControl();
+    if (node.active_ctx) {
+        node.active_ctx->Interrupt();
+    }
     if (node.llmq_ctx) {
         node.llmq_ctx->Interrupt();
     }
@@ -266,6 +269,7 @@ void PrepareShutdown(NodeContext& node)
     StopREST();
     StopRPC();
     StopHTTPServer();
+    if (node.active_ctx) node.active_ctx->Stop();
     if (node.llmq_ctx) node.llmq_ctx->Stop();
 
     for (const auto& client : node.chain_clients) {
@@ -2263,7 +2267,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 
     // ********************************************************* Step 10a: schedule Dash-specific tasks
 
-    node.llmq_ctx->Start(*node.connman, *node.peerman);
+    node.llmq_ctx->Start(*node.peerman);
+    if (node.active_ctx) node.active_ctx->Start(*node.connman, *node.peerman);
 
     node.scheduler->scheduleEvery(std::bind(&CNetFulfilledRequestManager::DoMaintenance, std::ref(*node.netfulfilledman)), std::chrono::minutes{1});
     node.scheduler->scheduleEvery(std::bind(&CMasternodeSync::DoMaintenance, std::ref(*node.mn_sync), std::cref(*node.peerman), std::cref(*node.govman)), std::chrono::seconds{1});

src/instantsend/signing.cpp

@@ -13,6 +13,7 @@
 
 #include <chainlock/chainlock.h>
 #include <llmq/quorums.h>
+#include <llmq/signing_shares.h>
 #include <masternode/sync.h>
 #include <spork.h>
 
@@ -330,7 +331,7 @@ bool InstantSendSigner::TrySignInputLocks(const CTransaction& tx, bool fRetroact
         WITH_LOCK(cs_input_requests, inputRequestIds.emplace(id));
         LogPrint(BCLog::INSTANTSEND, "%s -- txid=%s: trying to vote on input %s with id %s. fRetroactive=%d\n",
                  __func__, tx.GetHash().ToString(), in.prevout.ToStringShort(), id.ToString(), fRetroactive);
-        if (m_sigman.AsyncSignIfMember(llmqType, m_shareman, id, tx.GetHash(), {}, fRetroactive)) {
+        if (m_shareman.AsyncSignIfMember(llmqType, m_sigman, id, tx.GetHash(), {}, fRetroactive)) {
             LogPrint(BCLog::INSTANTSEND, "%s -- txid=%s: voted on input %s with id %s\n", __func__,
                      tx.GetHash().ToString(), in.prevout.ToStringShort(), id.ToString());
         }
@@ -388,6 +389,6 @@ void InstantSendSigner::TrySignInstantSendLock(const CTransaction& tx)
         txToCreatingInstantSendLocks.emplace(tx.GetHash(), &e.first->second);
     }
 
-    m_sigman.AsyncSignIfMember(llmqType, m_shareman, id, tx.GetHash(), quorum->m_quorum_base_block_index->GetBlockHash());
+    m_shareman.AsyncSignIfMember(llmqType, m_sigman, id, tx.GetHash(), quorum->m_quorum_base_block_index->GetBlockHash());
 }
 } // namespace instantsend

src/llmq/context.cpp

@@ -12,15 +12,13 @@
 #include <llmq/dkgsessionmgr.h>
 #include <llmq/quorums.h>
 #include <llmq/signing.h>
-#include <llmq/signing_shares.h>
 #include <llmq/snapshot.h>
 #include <validation.h>
 
 LLMQContext::LLMQContext(ChainstateManager& chainman, CDeterministicMNManager& dmnman, CEvoDB& evo_db,
                          CMasternodeMetaMan& mn_metaman, CMNHFManager& mnhfman, CSporkManager& sporkman,
                          CTxMemPool& mempool, const CActiveMasternodeManager* const mn_activeman,
                          const CMasternodeSync& mn_sync, bool unit_tests, bool wipe) :
-    is_masternode{mn_activeman != nullptr},
     bls_worker{std::make_shared<CBLSWorker>()},
     dkg_debugman{std::make_unique<llmq::CDKGDebugManager>()},
     qsnapman{std::make_unique<llmq::CQuorumSnapshotManager>(evo_db)},
@@ -32,10 +30,8 @@ LLMQContext::LLMQContext(ChainstateManager& chainman, CDeterministicMNManager& d
     qman{std::make_unique<llmq::CQuorumManager>(*bls_worker, chainman.ActiveChainstate(), dmnman, *qdkgsman, evo_db,
                                                 *quorum_block_processor, *qsnapman, mn_activeman, mn_sync, sporkman,
                                                 unit_tests, wipe)},
-    sigman{std::make_unique<llmq::CSigningManager>(mn_activeman, chainman.ActiveChainstate(), *qman, unit_tests, wipe)},
-    shareman{std::make_unique<llmq::CSigSharesManager>(*sigman, mn_activeman, *qman, sporkman)},
-    clhandler{std::make_unique<llmq::CChainLocksHandler>(chainman.ActiveChainstate(), *qman, *sigman, sporkman, mempool,
-                                                         mn_sync)},
+    sigman{std::make_unique<llmq::CSigningManager>(chainman.ActiveChainstate(), *qman, unit_tests, wipe)},
+    clhandler{std::make_unique<llmq::CChainLocksHandler>(chainman.ActiveChainstate(), *qman, sporkman, mempool, mn_sync)},
     isman{std::make_unique<llmq::CInstantSendManager>(*clhandler, chainman.ActiveChainstate(), *qman, *sigman, sporkman,
                                                       mempool, mn_sync, unit_tests, wipe)}
 {
@@ -49,31 +45,21 @@ LLMQContext::~LLMQContext() {
 
 void LLMQContext::Interrupt() {
     isman->InterruptWorkerThread();
-    shareman->InterruptWorkerThread();
     sigman->InterruptWorkerThread();
 }
 
-void LLMQContext::Start(CConnman& connman, PeerManager& peerman)
+void LLMQContext::Start(PeerManager& peerman)
 {
-    if (is_masternode) {
-        qdkgsman->StartThreads(connman, peerman);
-    }
     qman->Start();
     sigman->StartWorkerThread(peerman);
-    shareman->RegisterAsRecoveredSigsListener();
-    shareman->StartWorkerThread(connman, peerman);
     clhandler->Start(*isman);
     isman->Start(peerman);
 }
 
-void LLMQContext::Stop() {
+void LLMQContext::Stop()
+{
     isman->Stop();
     clhandler->Stop();
-    shareman->StopWorkerThread();
-    shareman->UnregisterAsRecoveredSigsListener();
     sigman->StopWorkerThread();
     qman->Stop();
-    if (is_masternode) {
-        qdkgsman->StopThreads();
-    }
 }

src/llmq/context.h

@@ -9,7 +9,6 @@
 
 class CActiveMasternodeManager;
 class CBLSWorker;
-class CConnman;
 class ChainstateManager;
 class CDeterministicMNManager;
 class CEvoDB;
@@ -28,14 +27,10 @@ class CInstantSendManager;
 class CQuorumBlockProcessor;
 class CQuorumManager;
 class CQuorumSnapshotManager;
-class CSigSharesManager;
 class CSigningManager;
 }
 
 struct LLMQContext {
-private:
-    const bool is_masternode;
-
 public:
     LLMQContext() = delete;
     LLMQContext(const LLMQContext&) = delete;
@@ -46,7 +41,7 @@ struct LLMQContext {
     ~LLMQContext();
 
     void Interrupt();
-    void Start(CConnman& connman, PeerManager& peerman);
+    void Start(PeerManager& peerman);
     void Stop();
 
     /** Guaranteed if LLMQContext is initialized then all members are valid too
@@ -66,7 +61,6 @@ struct LLMQContext {
     const std::unique_ptr<llmq::CDKGSessionManager> qdkgsman;
     const std::unique_ptr<llmq::CQuorumManager> qman;
     const std::unique_ptr<llmq::CSigningManager> sigman;
-    const std::unique_ptr<llmq::CSigSharesManager> shareman;
     const std::unique_ptr<llmq::CChainLocksHandler> clhandler;
     const std::unique_ptr<llmq::CInstantSendManager> isman;
 };

src/llmq/ehf_signals.cpp

@@ -3,17 +3,19 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <llmq/ehf_signals.h>
-#include <llmq/quorums.h>
-#include <llmq/commitment.h>
 
 #include <chainparams.h>
 #include <consensus/validation.h>
 #include <deploymentstatus.h>
-#include <evo/mnhftx.h>
 #include <index/txindex.h> // g_txindex
 #include <primitives/transaction.h>
 #include <validation.h>
 
+#include <evo/mnhftx.h>
+#include <llmq/commitment.h>
+#include <llmq/quorums.h>
+#include <llmq/signing_shares.h>
+
 namespace llmq {
 CEHFSignalsHandler::CEHFSignalsHandler(ChainstateManager& chainman, CMNHFManager& mnhfman, CSigningManager& sigman,
                                        CSigSharesManager& shareman, const CQuorumManager& qman) :
@@ -76,7 +78,7 @@ void CEHFSignalsHandler::trySignEHFSignal(int bit, const CBlockIndex* const pind
     const uint256 msgHash = mnhfPayload.PrepareTx().GetHash();
 
     WITH_LOCK(cs, ids.insert(requestId));
-    sigman.AsyncSignIfMember(llmqType, shareman, requestId, msgHash, quorum->qc->quorumHash, false, true);
+    shareman.AsyncSignIfMember(llmqType, sigman, requestId, msgHash, quorum->qc->quorumHash, false, true);
 }
 
 MessageProcessingResult CEHFSignalsHandler::HandleNewRecoveredSig(const CRecoveredSig& recoveredSig)

src/llmq/signing.cpp

@@ -332,10 +332,8 @@ void CRecoveredSigsDb::CleanupOldVotes(int64_t maxAge)
 
 //////////////////
 
-CSigningManager::CSigningManager(const CActiveMasternodeManager* const mn_activeman, const CChainState& chainstate,
-                                 const CQuorumManager& _qman, bool fMemory, bool fWipe) :
+CSigningManager::CSigningManager(const CChainState& chainstate, const CQuorumManager& _qman, bool fMemory, bool fWipe) :
     db(fMemory, fWipe),
-    m_mn_activeman(mn_activeman),
     m_chainstate(chainstate),
     qman(_qman)
 {
@@ -615,10 +613,6 @@ void CSigningManager::ProcessRecoveredSig(const std::shared_ptr<const CRecovered
     db.WriteRecoveredSig(*recoveredSig);
     WITH_LOCK(cs_pending, pendingReconstructedRecoveredSigs.erase(recoveredSig->GetHash()));
 
-    if (m_mn_activeman != nullptr) {
-        peerman.RelayRecoveredSig(recoveredSig->GetHash());
-    }
-
     auto listeners = WITH_LOCK(cs_listeners, return recoveredSigsListeners);
     for (auto& l : listeners) {
         peerman.PostProcessMessage(l->HandleNewRecoveredSig(*recoveredSig));
@@ -666,80 +660,6 @@ void CSigningManager::UnregisterRecoveredSigsListener(CRecoveredSigsListener* l)
     recoveredSigsListeners.erase(itRem, recoveredSigsListeners.end());
 }
 
-bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigSharesManager& shareman, const uint256& id,
-                                        const uint256& msgHash, const uint256& quorumHash, bool allowReSign,
-                                        bool allowDiffMsgHashSigning)
-{
-    if (m_mn_activeman == nullptr) return false;
-    if (m_mn_activeman->GetProTxHash().IsNull()) return false;
-
-    const auto quorum = [&]() {
-        if (quorumHash.IsNull()) {
-            // This might end up giving different results on different members
-            // This might happen when we are on the brink of confirming a new quorum
-            // This gives a slight risk of not getting enough shares to recover a signature
-            // But at least it shouldn't be possible to get conflicting recovered signatures
-            // TODO fix this by re-signing when the next block arrives, but only when that block results in a change of the quorum list and no recovered signature has been created in the mean time
-            const auto &llmq_params_opt = Params().GetLLMQ(llmqType);
-            assert(llmq_params_opt.has_value());
-            return SelectQuorumForSigning(llmq_params_opt.value(), m_chainstate.m_chain, qman, id);
-        } else {
-            return qman.GetQuorum(llmqType, quorumHash);
-        }
-    }();
-
-    if (!quorum) {
-        LogPrint(BCLog::LLMQ, "CSigningManager::%s -- failed to select quorum. id=%s, msgHash=%s\n", __func__, id.ToString(), msgHash.ToString());
-        return false;
-    }
-
-    if (!quorum->IsValidMember(m_mn_activeman->GetProTxHash())) {
-        return false;
-    }
-
-    {
-        bool hasVoted = db.HasVotedOnId(llmqType, id);
-        if (hasVoted) {
-            uint256 prevMsgHash;
-            db.GetVoteForId(llmqType, id, prevMsgHash);
-            if (msgHash != prevMsgHash) {
-                if (allowDiffMsgHashSigning) {
-                    LogPrintf("CSigningManager::%s -- already voted for id=%s and msgHash=%s. Signing for different msgHash=%s\n",
-                              __func__, id.ToString(), prevMsgHash.ToString(), msgHash.ToString());
-                    hasVoted = false;
-                } else {
-                    LogPrintf("CSigningManager::%s -- already voted for id=%s and msgHash=%s. Not voting on conflicting msgHash=%s\n",
-                              __func__, id.ToString(), prevMsgHash.ToString(), msgHash.ToString());
-                    return false;
-                }
-            } else if (allowReSign) {
-                LogPrint(BCLog::LLMQ, "CSigningManager::%s -- already voted for id=%s and msgHash=%s. Resigning!\n", __func__,
-                         id.ToString(), prevMsgHash.ToString());
-            } else {
-                LogPrint(BCLog::LLMQ, "CSigningManager::%s -- already voted for id=%s and msgHash=%s. Not voting again.\n", __func__,
-                          id.ToString(), prevMsgHash.ToString());
-                return false;
-            }
-        }
-
-        if (db.HasRecoveredSigForId(llmqType, id)) {
-            // no need to sign it if we already have a recovered sig
-            return true;
-        }
-        if (!hasVoted) {
-            db.WriteVoteForId(llmqType, id, msgHash);
-        }
-    }
-
-    if (allowReSign) {
-        // make us re-announce all known shares (other nodes might have run into a timeout)
-        shareman.ForceReAnnouncement(quorum, llmqType, id, msgHash);
-    }
-    shareman.AsyncSign(quorum, id, msgHash);
-
-    return true;
-}
-
 bool CSigningManager::HasRecoveredSig(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash) const
 {
     return db.HasRecoveredSig(llmqType, id, msgHash);
@@ -791,7 +711,7 @@ void CSigningManager::StartWorkerThread(PeerManager& peerman)
         assert(false);
     }
 
-    workThread = std::thread(&util::TraceThread, "sigshares", [this, &peerman] { WorkThreadMain(peerman); });
+    workThread = std::thread(&util::TraceThread, "recsigs", [this, &peerman] { WorkThreadMain(peerman); });
 }
 
 void CSigningManager::StopWorkerThread()