Merge #7115: refactor: separate network and consensus logic 2b/N [sig shares]

ce11a03 refactor: Replace GetAllNodes() + RemoveAsBanned() with single RemoveNodesIf(predicate) method (UdjinM6)
cfff7cb chore: typo (UdjinM6)
3da19f8 refactor: static class members to static anonymous functions of CSigningSharesManager (Konstantin Akimov)
fa655d8 refactor: split PreVerifyBatchedSigShares to non-banning helper PreVerifySigShareQuorum and banning helper (Konstantin Akimov)
bbc8f42 refactor: final removing of PeerManager from signing_shares (Konstantin Akimov)
29e4a87 refactor: move include msg_result.h from header to source for signing_shares and cleanup (Konstantin Akimov)
4bf0308 fix: CSigningSharesManager supposed to use _active_ chain but not the chain that has been active during it's creation (Konstantin Akimov)
b18eaf2 refactor: move CSigSharesManager::RemoveBannedNodeStates inside NetSigning (Konstantin Akimov)
b8fc3fb refactor: move ProcessPendingSigShares from signing_shares to NetSigning (Konstantin Akimov)
115fe9c refactor: remove duplicated code for p2p messages QSIGSHARESINV and QGETSIGSHARES (Konstantin Akimov)
2d485c5 move out BanNode from ProcessMessageSigShare (Konstantin Akimov)
5a13f03 refactor: move ProcessMessage from signing_shares to NetSigning (Konstantin Akimov)
41ee4fe refactor: drop direct dependency of PeerManager on llmq/signing_shares (Konstantin Akimov)
8a6bb8a refactor: simplify CEHFSignalsHandler::HandleNewRecoveredSig a bit (Konstantin Akimov)
3c1c8f0 refactor: move worker threads from signing_shares to NetSigning (Konstantin Akimov)
f8cd67e refactor: move listener NotifyRecoveredSig from llmq/signing_share to net_signing (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Separation of consensus & chain code and network & node in Dash Core is blocked by tight connection of network and consensus code; in components such as llmq::CSigningManager, llmq::CSigSharesManager, coinjoin::client, coinjoin::server, governance/, llmq::CInstantSendManager, etc.
  These dependencies on PeerManager blocks backport's of bitcoin related to 'kernel' project and makes circular dependencies over net_processing; it's also a blocker for bitcoin#25144

  ## What was done?
  This PR addresses a dependency of llmq::CSigSharesManager on PeerManager, it is continuation of #6992 which addressed llmq::CSigningManager

  It is a split from proof-of-concept PR #6934. Due to multiple other changes (review comments to prior PRs, ActiveContext-related refactorings, proactive signature pushing the implementation is not fully matched with original 6934, the patchset is re-created from scratch.

  Prior work:
   - #6959
   - #6992

  NOTE: this PR is not a direct blocker of chainstate / kernel so far as `CSigSharesManager` already aggregated to masternode's ActiveContext by kwvg, see #6841

  ## How Has This Been Tested?
  Run unit & functional tests
  Run linter lint-circular-dependencies.py

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] 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 _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK ce11a03

Tree-SHA512: 01740144f9115160b5b2cc79678a61a8946d4e8f759c954bb608f4fb78e064f6538222deb370b3b40939e15a98d7146d8d49ec5e50fd7d48af34f2ccda2a49a1

Author: PastaPastaPasta

src/active/context.cpp

@@ -29,16 +29,15 @@ ActiveContext::ActiveContext(CBLSWorker& bls_worker, ChainstateManager& chainman
                              chainlock::ChainlockHandler& clhandler, llmq::CInstantSendManager& isman,
                              llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumManager& qman,
                              llmq::CQuorumSnapshotManager& qsnapman, llmq::CSigningManager& sigman,
-                             PeerManager& peerman, const CMasternodeSync& mn_sync, const CBLSSecretKey& operator_sk,
+                             const CMasternodeSync& mn_sync, const CBLSSecretKey& operator_sk,
                              const llmq::QvvecSyncModeMap& sync_map, const util::DbWrapperParams& db_params,
                              bool quorums_recovery, bool quorums_watch) :
     m_isman{isman},
     m_qman{qman},
     nodeman{std::make_unique<CActiveMasternodeManager>(connman, dmnman, operator_sk)},
     dkgdbgman{std::make_unique<llmq::CDKGDebugManager>()},
     qdkgsman{std::make_unique<llmq::CDKGSessionManager>(dmnman, qsnapman, chainman, sporkman, db_params, quorums_watch)},
-    shareman{std::make_unique<llmq::CSigSharesManager>(connman, chainman.ActiveChainstate(), sigman, peerman, *nodeman,
-                                                       qman, sporkman)},
+    shareman{std::make_unique<llmq::CSigSharesManager>(connman, chainman, sigman, *nodeman, qman, sporkman)},
     gov_signer{std::make_unique<GovernanceSigner>(connman, dmnman, govman, *nodeman, chainman, mn_sync)},
     ehf_sighandler{std::make_unique<llmq::CEHFSignalsHandler>(chainman, sigman, *shareman, qman)},
     qman_handler{std::make_unique<llmq::QuorumParticipant>(bls_worker, connman, dmnman, qman, qsnapman, *nodeman, chainman,
@@ -64,20 +63,13 @@ ActiveContext::~ActiveContext()
     m_isman.DisconnectSigner();
 }
 
-void ActiveContext::Interrupt()
-{
-    shareman->InterruptWorkerThread();
-}
-
 void ActiveContext::Start(CConnman& connman, PeerManager& peerman)
 {
     qman_handler->Start();
     qdkgsman->StartThreads(connman, peerman);
-    shareman->Start();
     cl_signer->Start();
     cl_signer->RegisterRecoveryInterface();
     is_signer->RegisterRecoveryInterface();
-    shareman->RegisterRecoveryInterface();
 
     RegisterValidationInterface(cl_signer.get());
 }
@@ -86,11 +78,9 @@ void ActiveContext::Stop()
 {
     UnregisterValidationInterface(cl_signer.get());
 
-    shareman->UnregisterRecoveryInterface();
     is_signer->UnregisterRecoveryInterface();
     cl_signer->UnregisterRecoveryInterface();
     cl_signer->Stop();
-    shareman->Stop();
     qdkgsman->StopThreads();
     qman_handler->Stop();
 }
@@ -118,8 +108,3 @@ void ActiveContext::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIn
     qdkgsman->UpdatedBlockTip(pindexNew, fInitialDownload);
     qman_handler->UpdatedBlockTip(pindexNew, fInitialDownload);
 }
-
-void ActiveContext::NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& sig, bool proactive_relay)
-{
-    shareman->NotifyRecoveredSig(sig, proactive_relay);
-}

src/active/context.h

@@ -66,13 +66,12 @@ struct ActiveContext final : public CValidationInterface {
                            CSporkManager& sporkman, const chainlock::Chainlocks& chainlocks, CTxMemPool& mempool,
                            chainlock::ChainlockHandler& clhandler, llmq::CInstantSendManager& isman,
                            llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumManager& qman,
-                           llmq::CQuorumSnapshotManager& qsnapman, llmq::CSigningManager& sigman, PeerManager& peerman,
+                           llmq::CQuorumSnapshotManager& qsnapman, llmq::CSigningManager& sigman,
                            const CMasternodeSync& mn_sync, const CBLSSecretKey& operator_sk,
                            const llmq::QvvecSyncModeMap& sync_map, const util::DbWrapperParams& db_params,
                            bool quorums_recovery, bool quorums_watch);
     ~ActiveContext();
 
-    void Interrupt();
     void Start(CConnman& connman, PeerManager& peerman);
     void Stop();
 
@@ -81,7 +80,6 @@ struct ActiveContext final : public CValidationInterface {
 
 protected:
     // CValidationInterface
-    void NotifyRecoveredSig(const std::shared_ptr<const llmq::CRecoveredSig>& sig, bool proactive_relay) override;
     void UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) override;
 
 public:

src/init.cpp

@@ -253,9 +253,6 @@ void Interrupt(NodeContext& node)
     InterruptRPC();
     InterruptREST();
     InterruptTorControl();
-    if (node.active_ctx) {
-        node.active_ctx->Interrupt();
-    }
     if (node.peerman) {
         node.peerman->InterruptHandlers();
     }
@@ -2199,7 +2196,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         node.active_ctx = std::make_unique<ActiveContext>(*node.llmq_ctx->bls_worker, chainman, *node.connman, *node.dmnman, *node.govman, *node.mn_metaman,
                                                           *node.sporkman, *node.chainlocks, *node.mempool, *node.clhandler, *node.llmq_ctx->isman,
                                                           *node.llmq_ctx->quorum_block_processor, *node.llmq_ctx->qman, *node.llmq_ctx->qsnapman, *node.llmq_ctx->sigman,
-                                                          *node.peerman, *node.mn_sync, operator_sk, sync_map, dash_db_params, quorums_recovery, quorums_watch);
+                                                          *node.mn_sync, operator_sk, sync_map, dash_db_params, quorums_recovery, quorums_watch);
         RegisterValidationInterface(node.active_ctx.get());
     } else if (quorums_watch) {
         node.observer_ctx = std::make_unique<llmq::ObserverContext>(*node.llmq_ctx->bls_worker, *node.connman, *node.dmnman, *node.mn_metaman, *node.mn_sync,
@@ -2211,7 +2208,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     // ********************************************************* Step 7d: Setup other Dash services
 
     node.peerman->AddExtraHandler(std::make_unique<NetInstantSend>(node.peerman.get(), *node.llmq_ctx->isman, *node.llmq_ctx->qman, chainman.ActiveChainstate()));
-    node.peerman->AddExtraHandler(std::make_unique<NetSigning>(node.peerman.get(), *node.llmq_ctx->sigman));
+    node.peerman->AddExtraHandler(std::make_unique<llmq::NetSigning>(node.peerman.get(), *node.llmq_ctx->sigman, node.active_ctx ? node.active_ctx->shareman.get() : nullptr, *node.sporkman));
 
     if (node.active_ctx) {
         auto cj_server = std::make_unique<CCoinJoinServer>(node.peerman.get(), chainman, *node.connman, *node.dmnman, *node.dstxman, *node.mn_metaman,

src/llmq/ehf_signals.cpp

@@ -92,7 +92,6 @@ MessageProcessingResult CEHFSignalsHandler::HandleNewRecoveredSig(const CRecover
         return {};
     }
 
-    MessageProcessingResult ret;
     const auto ehfSignals = m_chainman.ActiveChainstate().ChainHelper().ehf_manager->GetSignalsStage(
         WITH_LOCK(::cs_main, return m_chainman.ActiveTip()));
     MNHFTxPayload mnhfPayload;
@@ -113,19 +112,20 @@ MessageProcessingResult CEHFSignalsHandler::HandleNewRecoveredSig(const CRecover
 
         CMutableTransaction tx = mnhfPayload.PrepareTx();
 
-        {
-            CTransactionRef tx_to_sent = MakeTransactionRef(std::move(tx));
-            LogPrintf("CEHFSignalsHandler::HandleNewRecoveredSig Special EHF TX is created hash=%s\n", tx_to_sent->GetHash().ToString());
-            LOCK(::cs_main);
-            const MempoolAcceptResult result = m_chainman.ProcessTransaction(tx_to_sent);
-            if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
-                ret.m_transactions.push_back(tx_to_sent->GetHash());
-            } else {
-                LogPrintf("CEHFSignalsHandler::HandleNewRecoveredSig -- AcceptToMemoryPool failed: %s\n", result.m_state.ToString());
-            }
+        CTransactionRef tx_to_sent = MakeTransactionRef(std::move(tx));
+        LogPrintf("CEHFSignalsHandler::HandleNewRecoveredSig Special EHF TX is created hash=%s\n",
+                  tx_to_sent->GetHash().ToString());
+        LOCK(::cs_main);
+        const MempoolAcceptResult result = m_chainman.ProcessTransaction(tx_to_sent);
+        if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
+            MessageProcessingResult ret;
+            ret.m_transactions.push_back(tx_to_sent->GetHash());
+            return ret;
         }
-        break;
+        LogPrintf("CEHFSignalsHandler::HandleNewRecoveredSig -- AcceptToMemoryPool failed: %s\n",
+                  result.m_state.ToString());
+        return {};
     }
-    return ret;
+    return {};
 }
 } // namespace llmq