refactor: separate masternode mode logic into dedicated manager class

Author: kwvg

src/Makefile.am

@@ -233,6 +233,7 @@ BITCOIN_CORE_H = \
   instantsend/db.h \
   instantsend/instantsend.h \
   instantsend/lock.h \
+  instantsend/signing.h \
   key.h \
   key_io.h \
   limitedmap.h \

src/instantsend/instantsend.cpp

@@ -19,6 +19,7 @@
 
 #include <bls/bls_batchverifier.h>
 #include <instantsend/lock.h>
+#include <instantsend/signing.h>
 #include <llmq/chainlocks.h>
 #include <llmq/commitment.h>
 #include <llmq/quorums.h>
@@ -51,11 +52,12 @@ CInstantSendManager::CInstantSendManager(CChainLocksHandler& _clhandler, CChainS
     m_chainstate{chainstate},
     qman{_qman},
     sigman{_sigman},
-    shareman{_shareman},
     spork_manager{sporkman},
     mempool{_mempool},
     m_mn_sync{mn_sync},
-    m_is_masternode{is_masternode}
+    m_signer{is_masternode ? std::make_unique<instantsend::InstantSendSigner>(chainstate, _clhandler, *this, _sigman,
+                                                                              _shareman, _qman, sporkman, _mempool, mn_sync)
+                           : nullptr}
 {
     workInterrupt.reset();
 }
@@ -71,12 +73,16 @@ void CInstantSendManager::Start(PeerManager& peerman)
 
     workThread = std::thread(&util::TraceThread, "isman", [this, &peerman] { WorkThreadMain(peerman); });
 
-    sigman.RegisterRecoveredSigsListener(this);
+    if (m_signer) {
+        sigman.RegisterRecoveredSigsListener(m_signer.get());
+    }
 }
 
 void CInstantSendManager::Stop()
 {
-    sigman.UnregisterRecoveredSigsListener(this);
+    if (m_signer) {
+        sigman.UnregisterRecoveredSigsListener(m_signer.get());
+    }
 
     // make sure to call InterruptWorkerThread() first
     if (!workInterrupt) {
@@ -337,10 +343,10 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, PeerManager& peerm
 {
     LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txid=%s, islock=%s: processing islock, peer=%d\n", __func__,
              islock->txid.ToString(), hash.ToString(), from);
-    {
-        LOCK(cs_creating);
-        creatingInstantSendLocks.erase(islock->GetRequestId());
-        txToCreatingInstantSendLocks.erase(islock->txid);
+    if (m_signer) {
+        LOCK(m_signer->cs_creating);
+        m_signer->creatingInstantSendLocks.erase(islock->GetRequestId());
+        m_signer->txToCreatingInstantSendLocks.erase(islock->txid);
     }
     if (db.KnownInstantSendLock(hash)) {
         return;
@@ -411,7 +417,7 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, PeerManager& peerm
         // bump mempool counter to make sure newly locked txes are picked up by getblocktemplate
         mempool.AddTransactionsUpdated(1);
     } else {
-        peerman.AskPeersForTransaction(islock->txid, m_is_masternode);
+        peerman.AskPeersForTransaction(islock->txid, /*is_masternode=*/m_signer != nullptr);
     }
 }
 
@@ -440,7 +446,9 @@ void CInstantSendManager::TransactionAddedToMempool(PeerManager& peerman, const
     }
 
     if (islock == nullptr) {
-        ProcessTx(*tx, false, Params().GetConsensus());
+        if (m_signer) {
+            m_signer->ProcessTx(*tx, false, Params().GetConsensus());
+        }
         // TX is not locked, so make sure it is tracked
         AddNonLockedTx(tx, nullptr);
     } else {
@@ -479,7 +487,9 @@ void CInstantSendManager::BlockConnected(const std::shared_ptr<const CBlock>& pb
             }
 
             if (!IsLocked(tx->GetHash()) && !has_chainlock) {
-                ProcessTx(*tx, true, Params().GetConsensus());
+                if (m_signer) {
+                    m_signer->ProcessTx(*tx, true, Params().GetConsensus());
+                }
                 // TX is not locked, so make sure it is tracked
                 AddNonLockedTx(tx, pindex);
             } else {
@@ -582,19 +592,22 @@ void CInstantSendManager::RemoveNonLockedTx(const uint256& txid, bool retryChild
 void CInstantSendManager::RemoveConflictedTx(const CTransaction& tx)
 {
     RemoveNonLockedTx(tx.GetHash(), false);
+    if (!m_signer) return;
 
-    LOCK(cs_inputReqests);
+    LOCK(m_signer->cs_inputReqests);
     for (const auto& in : tx.vin) {
         auto inputRequestId = ::SerializeHash(std::make_pair(INPUTLOCK_REQUESTID_PREFIX, in));
-        inputRequestIds.erase(inputRequestId);
+        m_signer->inputRequestIds.erase(inputRequestId);
     }
 }
 
 void CInstantSendManager::TruncateRecoveredSigsForInputs(const llmq::CInstantSendLock& islock)
 {
+    if (!m_signer) return;
+
     for (const auto& in : islock.inputs) {
         auto inputRequestId = ::SerializeHash(std::make_pair(INPUTLOCK_REQUESTID_PREFIX, in));
-        WITH_LOCK(cs_inputReqests, inputRequestIds.erase(inputRequestId));
+        WITH_LOCK(m_signer->cs_inputReqests, m_signer->inputRequestIds.erase(inputRequestId));
         sigman.TruncateRecoveredSig(Params().GetConsensus().llmqTypeDIP0024InstantSend, inputRequestId);
     }
 }
@@ -693,7 +706,7 @@ void CInstantSendManager::RemoveMempoolConflictsForLock(PeerManager& peerman, co
         for (const auto& p : toDelete) {
             RemoveConflictedTx(*p.second);
         }
-        peerman.AskPeersForTransaction(islock.txid, m_is_masternode);
+        peerman.AskPeersForTransaction(islock.txid, /*is_masternode=*/m_signer != nullptr);
     }
 }
 
@@ -898,7 +911,9 @@ void CInstantSendManager::WorkThreadMain(PeerManager& peerman)
 {
     while (!workInterrupt) {
         bool fMoreWork = ProcessPendingInstantSendLocks(peerman);
-        ProcessPendingRetryLockTxs();
+        if (m_signer) {
+            m_signer->ProcessPendingRetryLockTxs();
+        }
 
         if (!fMoreWork && !workInterrupt.sleep_for(std::chrono::milliseconds(100))) {
             return;

src/instantsend/instantsend.h

@@ -30,14 +30,18 @@ class CMasternodeSync;
 class CSporkManager;
 class PeerManager;
 
+namespace instantsend {
+class InstantSendSigner;
+} // namespace instantsend
+
 namespace llmq
 {
 class CChainLocksHandler;
 class CQuorumManager;
 class CSigningManager;
 class CSigSharesManager;
 
-class CInstantSendManager : public CRecoveredSigsListener
+class CInstantSendManager
 {
 private:
     instantsend::CInstantSendDb db;
@@ -46,35 +50,15 @@ class CInstantSendManager : 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<instantsend::InstantSendSigner> m_signer{nullptr};
 
     std::thread workThread;
     CThreadInterrupt workInterrupt;
 
-    mutable Mutex cs_inputReqests;
-
-    /**
-     * Request ids of inputs that we signed. Used to determine if a recovered signature belongs to an
-     * in-progress input lock.
-     */
-    std::unordered_set<uint256, StaticSaltedHasher> inputRequestIds GUARDED_BY(cs_inputReqests);
-
-    mutable Mutex cs_creating;
-
-    /**
-     * These are the islocks that are currently in the middle of being created. Entries are created when we observed
-     * recovered signatures for all inputs of a TX. At the same time, we initiate signing of our sigshare for the islock.
-     * When the recovered sig for the islock later arrives, we can finish the islock and propagate it.
-     */
-    std::unordered_map<uint256, CInstantSendLock, StaticSaltedHasher> creatingInstantSendLocks GUARDED_BY(cs_creating);
-    // maps from txid to the in-progress islock
-    std::unordered_map<uint256, CInstantSendLock*, StaticSaltedHasher> txToCreatingInstantSendLocks GUARDED_BY(cs_creating);
-
     mutable Mutex cs_pendingLocks;
     // Incoming and not verified yet
     std::unordered_map<uint256, std::pair<NodeId, CInstantSendLockPtr>, StaticSaltedHasher> pendingInstantSendLocks GUARDED_BY(cs_pendingLocks);
@@ -111,69 +95,52 @@ class CInstantSendManager : public CRecoveredSigsListener
     void InterruptWorkerThread() { workInterrupt(); };
 
 private:
-    void ProcessTx(const CTransaction& tx, bool fRetroactive, const Consensus::Params& params)
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests);
-    bool CheckCanLock(const CTransaction& tx, bool printDebug, const Consensus::Params& params) const;
-    bool CheckCanLock(const COutPoint& outpoint, bool printDebug, const uint256& txHash,
-                      const Consensus::Params& params) const;
-
-    void HandleNewInputLockRecoveredSig(const CRecoveredSig& recoveredSig, const uint256& txid)
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_creating);
-    void HandleNewInstantSendLockRecoveredSig(const CRecoveredSig& recoveredSig)
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_pendingLocks);
-
-    bool TrySignInputLocks(const CTransaction& tx, bool allowResigning, Consensus::LLMQType llmqType,
-                           const Consensus::Params& params) EXCLUSIVE_LOCKS_REQUIRED(!cs_inputReqests);
-    void TrySignInstantSendLock(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(!cs_creating);
-
     PeerMsgRet ProcessMessageInstantSendLock(const CNode& pfrom, PeerManager& peerman, const CInstantSendLockPtr& islock);
     bool ProcessPendingInstantSendLocks(PeerManager& peerman)
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests, !cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
+        EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
 
     std::unordered_set<uint256, StaticSaltedHasher> ProcessPendingInstantSendLocks(
         const Consensus::LLMQParams& llmq_params, PeerManager& peerman, int signOffset,
         const std::unordered_map<uint256, std::pair<NodeId, CInstantSendLockPtr>, StaticSaltedHasher>& pend, bool ban)
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests, !cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
+        EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
     void ProcessInstantSendLock(NodeId from, PeerManager& peerman, const uint256& hash, const CInstantSendLockPtr& islock)
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests, !cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
+        EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
 
     void AddNonLockedTx(const CTransactionRef& tx, const CBlockIndex* pindexMined)
         EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks);
     void RemoveNonLockedTx(const uint256& txid, bool retryChildren)
         EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry);
     void RemoveConflictedTx(const CTransaction& tx)
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_inputReqests, !cs_nonLocked, !cs_pendingRetry);
-    void TruncateRecoveredSigsForInputs(const CInstantSendLock& islock)
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_inputReqests);
+        EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry);
+    void TruncateRecoveredSigsForInputs(const CInstantSendLock& islock);
 
     void RemoveMempoolConflictsForLock(PeerManager& peerman, const uint256& hash, const CInstantSendLock& islock)
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_inputReqests, !cs_nonLocked, !cs_pendingRetry);
+        EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry);
     void ResolveBlockConflicts(const uint256& islockHash, const CInstantSendLock& islock)
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_inputReqests, !cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
-    void ProcessPendingRetryLockTxs()
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests, !cs_nonLocked, !cs_pendingRetry);
+        EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
 
     void WorkThreadMain(PeerManager& peerman)
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests, !cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
+        NO_THREAD_SAFETY_ANALYSIS;
+        // Needed as compiler cannot differentiate between negative capability against member and against
+        // member accessed through reference.
+        // TODO: Remove this, it's terrible.
+        // EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
 
     void HandleFullyConfirmedBlock(const CBlockIndex* pindex)
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_inputReqests, !cs_nonLocked, !cs_pendingRetry);
+        EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry);
 
 public:
     bool IsLocked(const uint256& txHash) const;
     bool IsWaitingForTx(const uint256& txHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingLocks);
     CInstantSendLockPtr GetConflictingLock(const CTransaction& tx) const;
 
-    [[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests, !cs_pendingLocks);
-
     PeerMsgRet ProcessMessage(const CNode& pfrom, PeerManager& peerman, std::string_view msg_type, CDataStream& vRecv);
 
     void TransactionAddedToMempool(PeerManager& peerman, const CTransactionRef& tx)
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests, !cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
+        EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
     void TransactionRemovedFromMempool(const CTransactionRef& tx);
     void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindex)
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_creating, !cs_inputReqests, !cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
+        EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingLocks, !cs_pendingRetry);
     void BlockDisconnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexDisconnected);
 
     bool AlreadyHave(const CInv& inv) const EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingLocks);
@@ -182,9 +149,9 @@ class CInstantSendManager : public CRecoveredSigsListener
     CInstantSendLockPtr GetInstantSendLockByTxid(const uint256& txid) const;
 
     void NotifyChainLock(const CBlockIndex* pindexChainLock)
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_inputReqests, !cs_nonLocked, !cs_pendingRetry);
+        EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry);
     void UpdatedBlockTip(const CBlockIndex* pindexNew)
-        EXCLUSIVE_LOCKS_REQUIRED(!cs_inputReqests, !cs_nonLocked, !cs_pendingRetry);
+        EXCLUSIVE_LOCKS_REQUIRED(!cs_nonLocked, !cs_pendingRetry);
 
     void RemoveConflictingLock(const uint256& islockHash, const CInstantSendLock& islock);
 
@@ -196,8 +163,9 @@ class CInstantSendManager : public CRecoveredSigsListener
      * transactions in mempool, but should sign txes included in a block. This
      * allows ChainLocks to continue even while this spork is disabled.
      */
-    bool IsInstantSendMempoolSigningEnabled() const;
     bool RejectConflictingBlocks() const;
+
+    friend class ::instantsend::InstantSendSigner;
 };
 } // namespace llmq