refactor: separate masternode mode logic into dedicated manager class

Author: kwvg

src/Makefile.am

@@ -152,6 +152,7 @@ BITCOIN_CORE_H = \
   chain.h \
   chainlock/chainlock.h \
   chainlock/clsig.h \
+  chainlock/signing.h \
   chainparams.h \
   chainparamsbase.h \
   chainparamsseeds.h \

src/chainlock/chainlock.cpp

@@ -16,6 +16,7 @@
 #include <validation.h>
 #include <validationinterface.h>
 
+#include <chainlock/signing.h>
 #include <instantsend/instantsend.h>
 #include <llmq/quorums.h>
 #include <masternode/sync.h>
@@ -34,17 +35,18 @@ namespace llmq {
 CChainLocksHandler::CChainLocksHandler(CChainState& chainstate, CQuorumManager& _qman, CSigningManager& _sigman,
                                        CSigSharesManager& _shareman, CSporkManager& sporkman, CTxMemPool& _mempool,
                                        const CMasternodeSync& mn_sync, bool is_masternode) :
-    m_chainstate(chainstate),
-    qman(_qman),
-    sigman(_sigman),
-    shareman(_shareman),
-    spork_manager(sporkman),
-    mempool(_mempool),
-    m_mn_sync(mn_sync),
-    m_is_masternode{is_masternode},
-    scheduler(std::make_unique<CScheduler>()),
-    scheduler_thread(
-        std::make_unique<std::thread>(std::thread(util::TraceThread, "cl-schdlr", [&] { scheduler->serviceQueue(); })))
+    m_chainstate{chainstate},
+    qman{_qman},
+    sigman{_sigman},
+    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}
 {
 }
 
@@ -56,19 +58,26 @@ CChainLocksHandler::~CChainLocksHandler()
 
 void CChainLocksHandler::Start(const llmq::CInstantSendManager& isman)
 {
-    sigman.RegisterRecoveredSigsListener(this);
+    if (m_signer) {
+        sigman.RegisterRecoveredSigsListener(m_signer.get());
+    }
     scheduler->scheduleEvery([&]() {
         CheckActiveState();
         EnforceBestChainLock();
+        Cleanup();
         // regularly retry signing the current chaintip as it might have failed before due to missing islocks
-        TrySignChainTip(isman);
+        if (m_signer) {
+            m_signer->TrySignChainTip(isman);
+        }
     }, std::chrono::seconds{5});
 }
 
 void CChainLocksHandler::Stop()
 {
     scheduler->stop();
-    sigman.UnregisterRecoveredSigsListener(this);
+    if (m_signer) {
+        sigman.UnregisterRecoveredSigsListener(m_signer.get());
+    }
 }
 
 bool CChainLocksHandler::AlreadyHave(const CInv& inv) const
@@ -195,7 +204,10 @@ void CChainLocksHandler::UpdatedBlockTip(const llmq::CInstantSendManager& isman)
         scheduler->scheduleFromNow([&]() {
             CheckActiveState();
             EnforceBestChainLock();
-            TrySignChainTip(isman);
+            Cleanup();
+            if (m_signer) {
+                m_signer->TrySignChainTip(isman);
+            }
             tryLockChainTipScheduled = false;
         }, std::chrono::seconds{0});
     }
@@ -234,36 +246,40 @@ void CChainLocksHandler::BlockConnected(const std::shared_ptr<const CBlock>& pbl
     }
 
     // We listen for BlockConnected so that we can collect all TX ids of all included TXs of newly received blocks
-    // We need this information later when we try to sign a new tip, so that we can determine if all included TXs are
-    // safe.
-
-    LOCK(cs);
-
-    auto it = blockTxs.find(pindex->GetBlockHash());
-    if (it == blockTxs.end()) {
-        // we must create this entry even if there are no lockable transactions in the block, so that TrySignChainTip
-        // later knows about this block
-        it = blockTxs.emplace(pindex->GetBlockHash(), std::make_shared<std::unordered_set<uint256, StaticSaltedHasher>>()).first;
-    }
-    auto& txids = *it->second;
-
     int64_t curTime = GetTime<std::chrono::seconds>().count();
-
-    for (const auto& tx : pblock->vtx) {
-        if (tx->IsCoinBase() || tx->vin.empty()) {
-            continue;
+    {
+        LOCK(cs);
+        for (const auto& tx : pblock->vtx) {
+            if (!tx->IsCoinBase() && !tx->vin.empty()) {
+                txFirstSeenTime.emplace(tx->GetHash(), curTime);
+            }
         }
-
-        txids.emplace(tx->GetHash());
-        txFirstSeenTime.emplace(tx->GetHash(), curTime);
     }
 
+    // We need this information later when we try to sign a new tip, so that we can determine if all included TXs are safe.
+    if (m_signer) {
+        LOCK(m_signer->cs_signer);
+        auto it = m_signer->blockTxs.find(pindex->GetBlockHash());
+        if (it == m_signer->blockTxs.end()) {
+            // We must create this entry even if there are no lockable transactions in the block, so that TrySignChainTip
+            // later knows about this block
+            it = m_signer->blockTxs.emplace(pindex->GetBlockHash(), std::make_shared<std::unordered_set<uint256, StaticSaltedHasher>>()).first;
+        }
+        auto& txids = *it->second;
+        for (const auto& tx : pblock->vtx) {
+            if (!tx->IsCoinBase() && !tx->vin.empty()) {
+                txids.emplace(tx->GetHash());
+            }
+        }
+    }
 }
 
 void CChainLocksHandler::BlockDisconnected(const std::shared_ptr<const CBlock>& pblock, gsl::not_null<const CBlockIndex*> pindexDisconnected)
 {
-    LOCK(cs);
-    blockTxs.erase(pindexDisconnected->GetBlockHash());
+    if (m_signer) {
+        LOCK(m_signer->cs_signer);
+        m_signer->blockTxs.erase(pindexDisconnected->GetBlockHash());
+    }
 }
 
 bool CChainLocksHandler::IsTxSafeForMining(const uint256& txid) const
@@ -428,15 +444,18 @@ void CChainLocksHandler::Cleanup()
         }
     }
 
-    LOCK(cs);
-    for (const auto& tx : CleanupSigner()) {
-        for (const auto& txid : *tx) {
-            txFirstSeenTime.erase(txid);
+    if (m_signer) {
+        const auto cleanup_txes{m_signer->Cleanup()};
+        LOCK(cs);
+        for (const auto& tx : cleanup_txes) {
+            for (const auto& txid : *tx) {
+                txFirstSeenTime.erase(txid);
+            }
         }
     }
 
-    // need mempool.cs due to GetTransaction calls
-    LOCK2(::cs_main, mempool.cs);
+    LOCK(::cs_main);
+    LOCK2(mempool.cs, cs); // need mempool.cs due to GetTransaction calls
     for (auto it = txFirstSeenTime.begin(); it != txFirstSeenTime.end(); ) {
         uint256 hashBlock;
         if (auto tx = GetTransaction(nullptr, &mempool, it->first, Params().GetConsensus(), hashBlock); !tx) {

src/chainlock/chainlock.h

@@ -7,11 +7,8 @@
 
 #include <chainlock/clsig.h>
 
-#include <crypto/common.h> // For ReadLE64
-#include <llmq/signing.h>
-#include <net.h> // For NodeId
-#include <net_types.h>
 #include <primitives/transaction.h>
+#include <protocol.h>
 #include <saltedhasher.h>
 #include <sync.h>
 
@@ -28,15 +25,20 @@ class CMasternodeSync;
 class CScheduler;
 class CSporkManager;
 class CTxMemPool;
+namespace chainlock {
+class ChainLockSigner;
+} // namespace chainlock
 
-namespace llmq
-{
+using NodeId = int64_t;
+
+namespace llmq {
 class CInstantSendManager;
+class CQuorumManager;
 class CSigningManager;
 class CSigSharesManager;
 enum class VerifyRecSigStatus;
 
-class CChainLocksHandler : public CRecoveredSigsListener
+class CChainLocksHandler
 {
     static constexpr int64_t CLEANUP_INTERVAL = 1000 * 30;
     static constexpr int64_t CLEANUP_SEEN_TIMEOUT = 24 * 60 * 60 * 1000;
@@ -48,14 +50,14 @@ class CChainLocksHandler : public CRecoveredSigsListener
     CChainState& m_chainstate;
     CQuorumManager& qman;
     CSigningManager& sigman;
-    CSigSharesManager& shareman;
     CSporkManager& spork_manager;
     CTxMemPool& mempool;
     const CMasternodeSync& m_mn_sync;
-
-    const bool m_is_masternode;
     std::unique_ptr<CScheduler> scheduler;
     std::unique_ptr<std::thread> scheduler_thread;
+
+    std::unique_ptr<chainlock::ChainLockSigner> m_signer{nullptr};
+
     mutable Mutex cs;
     std::atomic<bool> tryLockChainTipScheduled{false};
     std::atomic<bool> isEnabled{false};
@@ -67,17 +69,6 @@ class CChainLocksHandler : public CRecoveredSigsListener
     const CBlockIndex* bestChainLockBlockIndex GUARDED_BY(cs) {nullptr};
     const CBlockIndex* lastNotifyChainLockBlockIndex GUARDED_BY(cs) {nullptr};
 
-    int32_t lastSignedHeight GUARDED_BY(cs) {-1};
-    uint256 lastSignedRequestId GUARDED_BY(cs);
-    uint256 lastSignedMsgHash GUARDED_BY(cs);
-
-    // We keep track of txids from recently received blocks so that we can check if all TXs got islocked
-    struct BlockHasher
-    {
-        size_t operator()(const uint256& hash) const { return ReadLE64(hash.begin()); }
-    };
-    using BlockTxs = std::unordered_map<uint256, std::shared_ptr<std::unordered_set<uint256, StaticSaltedHasher>>, BlockHasher>;
-    BlockTxs blockTxs GUARDED_BY(cs);
     std::unordered_map<uint256, int64_t, StaticSaltedHasher> txFirstSeenTime GUARDED_BY(cs);
 
     std::map<uint256, int64_t> seenChainLocks GUARDED_BY(cs);
@@ -106,10 +97,7 @@ class CChainLocksHandler : public CRecoveredSigsListener
     void BlockConnected(const std::shared_ptr<const CBlock>& pblock, gsl::not_null<const CBlockIndex*> pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs);
     void BlockDisconnected(const std::shared_ptr<const CBlock>& pblock, gsl::not_null<const CBlockIndex*> pindexDisconnected) EXCLUSIVE_LOCKS_REQUIRED(!cs);
     void CheckActiveState() EXCLUSIVE_LOCKS_REQUIRED(!cs);
-    void TrySignChainTip(const llmq::CInstantSendManager& isman) EXCLUSIVE_LOCKS_REQUIRED(!cs);
     void EnforceBestChainLock() EXCLUSIVE_LOCKS_REQUIRED(!cs);
-    [[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override
-        EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
     bool HasChainLock(int nHeight, const uint256& blockHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
     bool HasConflictingChainLock(int nHeight, const uint256& blockHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
@@ -122,11 +110,9 @@ class CChainLocksHandler : public CRecoveredSigsListener
     bool InternalHasChainLock(int nHeight, const uint256& blockHash) const EXCLUSIVE_LOCKS_REQUIRED(cs);
     bool InternalHasConflictingChainLock(int nHeight, const uint256& blockHash) const EXCLUSIVE_LOCKS_REQUIRED(cs);
 
-    BlockTxs::mapped_type GetBlockTxs(const uint256& blockHash) EXCLUSIVE_LOCKS_REQUIRED(!cs);
-
     void Cleanup() EXCLUSIVE_LOCKS_REQUIRED(!cs);
-    std::vector<std::shared_ptr<std::unordered_set<uint256, StaticSaltedHasher>>> CleanupSigner()
-        EXCLUSIVE_LOCKS_REQUIRED(cs);
+
+    friend class ::chainlock::ChainLockSigner;
 };
 
 bool AreChainLocksEnabled(const CSporkManager& sporkman);