Merge #6838: refactor: split off governance masternode-only logic to GovernanceSigner, drop Relay()s and use periodic relay instead, minor cleanup

b51cd1d lint: apply most `clang-format` suggestions, update circular allowlist (Kittywhiskers Van Gogh)
700e069 refactor: cleanup headers and forward decls (Kittywhiskers Van Gogh)
c516fd3 refactor: apply review suggestions (Kittywhiskers Van Gogh)
d19e1f3 fix: add `nullptr` check before using `FindGovernanceObject()` retval (Kittywhiskers Van Gogh)
1777919 refactor: use `std::chrono` for governance time constants (Kittywhiskers Van Gogh)
4d96f5f refactor: move `DoMaintenance()` inside `Schedule()` (Kittywhiskers Van Gogh)
e224991 governance: add lock annotations for `cs_relay` (Kittywhiskers Van Gogh)
df589c7 governance: introduce task for relaying governance objects (Kittywhiskers Van Gogh)
3811924 governance: drop `Relay()` from `CGovernance{Object,Vote}` (Kittywhiskers Van Gogh)
63448ff refactor: abstract away parent implementation from signer (Kittywhiskers Van Gogh)
757ded3 refactor: remove need for access to private members (Kittywhiskers Van Gogh)
bd6af83 refactor: separate masternode mode logic into dedicated signer class (Kittywhiskers Van Gogh)
2b74e15 refactor: move masternode mode logic to `governance/signing.cpp` (Kittywhiskers Van Gogh)
fefbe27 refactor: replace `Sign` functions with direct activeman call (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Depends on #6828

  * Depends on #6842

  * Depends on #6877

  * Dependency for #6849

  * To reduce the proliferation of `PeerManager` in governance logic, relaying has been centralized to `CGovernanceManager` and now done through a scheduled task that processes a queue of requests. This reduces the amount of code that needs to fetch a reference to `PeerManager` in order to queue a message for relay and brings us closer to dropping it as a member in other objects.
    * On test chains, the queue of requests is processed every second but on mainnet, it is processed every 5 seconds. This is because functional tests assume instantaneous relay and fail if it takes too long to relay messages (as cascading delays cause test timeout).

  * Likewise, to reduce the references to `CActiveMasternodeManager`, signing logic is consolidated to `SignBasic()` with the objects using simple getter/setters (or direct assignment) where applicable.

  ## 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 b51cd1d
  knst:
    utACK  b51cd1d

Tree-SHA512: cd4bba1e7314695a215a8a83e2d5319be8d2eb2e88eb42f641f6c88ca6d74ee68ced9e95475c6b1d93f5b061993efa6a195d23f61a8775f920867d9836a0dcf8

Author: PastaPastaPasta

src/Makefile.am

@@ -222,11 +222,12 @@ BITCOIN_CORE_H = \
   evo/specialtxman.h \
   evo/types.h \
   dsnotificationinterface.h \
-  governance/governance.h \
   governance/classes.h \
   governance/common.h \
+  governance/governance.h \
   governance/exceptions.h \
   governance/object.h \
+  governance/signing.h \
   governance/validators.h \
   governance/vote.h \
   governance/votedb.h \
@@ -510,6 +511,7 @@ libbitcoin_node_a_SOURCES = \
   governance/exceptions.cpp \
   governance/governance.cpp \
   governance/object.cpp \
+  governance/signing.cpp \
   governance/validators.cpp \
   governance/vote.cpp \
   governance/votedb.cpp \

src/coinjoin/coinjoin.cpp

@@ -16,7 +16,6 @@
 #include <bls/bls.h>
 #include <chainlock/chainlock.h>
 #include <instantsend/instantsend.h>
-#include <masternode/node.h>
 #include <masternode/sync.h>
 
 #include <string>
@@ -45,18 +44,6 @@ uint256 CCoinJoinQueue::GetSignatureHash() const
 }
 uint256 CCoinJoinQueue::GetHash() const { return SerializeHash(*this, SER_NETWORK, PROTOCOL_VERSION); }
 
-bool CCoinJoinQueue::Sign(const CActiveMasternodeManager& mn_activeman)
-{
-    uint256 hash = GetSignatureHash();
-    CBLSSignature sig = mn_activeman.Sign(hash, /*is_legacy=*/ false);
-    if (!sig.IsValid()) {
-        return false;
-    }
-    vchSig = sig.ToByteVector(false);
-
-    return true;
-}
-
 bool CCoinJoinQueue::CheckSignature(const CBLSPublicKey& blsPubKey) const
 {
     if (!CBLSSignature(Span{vchSig}, false).VerifyInsecure(blsPubKey, GetSignatureHash(), false)) {
@@ -84,18 +71,6 @@ uint256 CCoinJoinBroadcastTx::GetSignatureHash() const
     return SerializeHash(*this, SER_GETHASH, PROTOCOL_VERSION);
 }
 
-bool CCoinJoinBroadcastTx::Sign(const CActiveMasternodeManager& mn_activeman)
-{
-    uint256 hash = GetSignatureHash();
-    CBLSSignature sig = mn_activeman.Sign(hash, /*is_legacy=*/ false);
-    if (!sig.IsValid()) {
-        return false;
-    }
-    vchSig = sig.ToByteVector(false);
-
-    return true;
-}
-
 bool CCoinJoinBroadcastTx::CheckSignature(const CBLSPublicKey& blsPubKey) const
 {
     if (!CBLSSignature(Span{vchSig}, false).VerifyInsecure(blsPubKey, GetSignatureHash(), false)) {

src/coinjoin/coinjoin.h

@@ -22,14 +22,12 @@
 #include <optional>
 #include <utility>
 
-class CActiveMasternodeManager;
 class CChainState;
 class CBLSPublicKey;
 class CBlockIndex;
 class ChainstateManager;
 class CMasternodeSync;
 class CTxMemPool;
-class TxValidationState;
 
 namespace llmq {
 class CChainLocksHandler;
@@ -209,14 +207,7 @@ class CCoinJoinQueue
 
     [[nodiscard]] uint256 GetHash() const;
     [[nodiscard]] uint256 GetSignatureHash() const;
-    /** Sign this mixing transaction
-     *  return true if all conditions are met:
-     *     1) we have an active Masternode,
-     *     2) we have a valid Masternode private key,
-     *     3) we signed the message successfully, and
-     *     4) we verified the message successfully
-     */
-    bool Sign(const CActiveMasternodeManager& mn_activeman);
+
     /// Check if we have a valid Masternode address
     [[nodiscard]] bool CheckSignature(const CBLSPublicKey& blsPubKey) const;
 
@@ -284,7 +275,6 @@ class CCoinJoinBroadcastTx
 
     [[nodiscard]] uint256 GetSignatureHash() const;
 
-    bool Sign(const CActiveMasternodeManager& mn_activeman);
     [[nodiscard]] bool CheckSignature(const CBLSPublicKey& blsPubKey) const;
 
     // Used only for unit tests

src/coinjoin/server.cpp

@@ -345,7 +345,7 @@ void CCoinJoinServer::CommitFinalTransaction()
     if (!m_dstxman.GetDSTX(hashTx)) {
         CCoinJoinBroadcastTx dstxNew(finalTransaction, m_mn_activeman.GetOutPoint(), m_mn_activeman.GetProTxHash(),
                                      GetAdjustedTime());
-        dstxNew.Sign(m_mn_activeman);
+        dstxNew.vchSig = m_mn_activeman.SignBasic(dstxNew.GetSignatureHash());
         m_dstxman.AddDSTX(dstxNew);
     }
 
@@ -504,7 +504,7 @@ void CCoinJoinServer::CheckForCompleteQueue()
                            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);
+        dsq.vchSig = m_mn_activeman.SignBasic(dsq.GetSignatureHash());
         m_peerman.RelayDSQ(dsq);
         WITH_LOCK(cs_vecqueue, vecCoinJoinQueue.push_back(dsq));
     }
@@ -714,7 +714,7 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage&
         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);
+        dsq.vchSig = m_mn_activeman.SignBasic(dsq.GetSignatureHash());
         m_peerman.RelayDSQ(dsq);
         LOCK(cs_vecqueue);
         vecCoinJoinQueue.push_back(dsq);

src/dsnotificationinterface.cpp

@@ -24,18 +24,14 @@
 CDSNotificationInterface::CDSNotificationInterface(CConnman& connman,
                                                    CMasternodeSync& mn_sync,
                                                    CGovernanceManager& govman,
-                                                   PeerManager& peerman,
                                                    const ChainstateManager& chainman,
-                                                   const CActiveMasternodeManager* const mn_activeman,
                                                    const std::unique_ptr<CDeterministicMNManager>& dmnman,
                                                    const std::unique_ptr<LLMQContext>& llmq_ctx,
                                                    const std::unique_ptr<CJContext>& cj_ctx)
   : m_connman(connman),
     m_mn_sync(mn_sync),
     m_govman(govman),
-    m_peerman(peerman),
     m_chainman(chainman),
-    m_mn_activeman(mn_activeman),
     m_dmnman(dmnman),
     m_llmq_ctx(llmq_ctx),
     m_cj_ctx(cj_ctx) {}
@@ -94,7 +90,7 @@ void CDSNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, con
     m_llmq_ctx->qdkgsman->UpdatedBlockTip(pindexNew, fInitialDownload);
 
     if (m_govman.IsValid()) {
-        m_govman.UpdatedBlockTip(pindexNew, m_connman, m_peerman, m_mn_activeman);
+        m_govman.UpdatedBlockTip(pindexNew);
     }
 }
 

src/dsnotificationinterface.h

@@ -7,13 +7,11 @@
 
 #include <validationinterface.h>
 
-class CActiveMasternodeManager;
 class CConnman;
 class CDeterministicMNManager;
 class CGovernanceManager;
 class ChainstateManager;
 class CMasternodeSync;
-class PeerManager;
 struct CJContext;
 struct LLMQContext;
 
@@ -23,9 +21,7 @@ class CDSNotificationInterface : public CValidationInterface
     explicit CDSNotificationInterface(CConnman& connman,
                                       CMasternodeSync& mn_sync,
                                       CGovernanceManager& govman,
-                                      PeerManager& peerman,
                                       const ChainstateManager& chainman,
-                                      const CActiveMasternodeManager* const mn_activeman,
                                       const std::unique_ptr<CDeterministicMNManager>& dmnman,
                                       const std::unique_ptr<LLMQContext>& llmq_ctx,
                                       const std::unique_ptr<CJContext>& cj_ctx);
@@ -52,9 +48,7 @@ class CDSNotificationInterface : public CValidationInterface
     CConnman& m_connman;
     CMasternodeSync& m_mn_sync;
     CGovernanceManager& m_govman;
-    PeerManager& m_peerman;
     const ChainstateManager& m_chainman;
-    const CActiveMasternodeManager* const m_mn_activeman;
     const std::unique_ptr<CDeterministicMNManager>& m_dmnman;
     const std::unique_ptr<LLMQContext>& m_llmq_ctx;
     const std::unique_ptr<CJContext>& m_cj_ctx;