evo/llmq/spork: Fix various (potential) locking issues (#3829)

* Fix potential deadlock in `CSporkManager::UpdateSpork()`

* Protect `inputRequestIds` with cs lock

* Protect `curDBTransaction` in `CEvoDB::CommitRootTransaction()`

* Check for `AssertLockNotHeld` in `EnforceBestChainLock()` instead of just having a comment in code

* Protect spork maps on (de)serialization

src/evo/evodb.cpp

@@ -53,6 +53,7 @@ void CEvoDB::RollbackCurTransaction()
 
 bool CEvoDB::CommitRootTransaction()
 {
+    LOCK(cs);
     assert(curDBTransaction.IsClean());
     rootDBTransaction.Commit();
     bool ret = db.WriteBatch(rootBatch);

src/llmq/quorums_chainlocks.cpp

@@ -488,6 +488,9 @@ bool CChainLocksHandler::IsTxSafeForMining(const uint256& txid)
 // This should also not be called from validation signals, as this might result in recursive calls
 void CChainLocksHandler::EnforceBestChainLock()
 {
+    AssertLockNotHeld(cs);
+    AssertLockNotHeld(cs_main);
+
     CChainLockSig clsig;
     const CBlockIndex* pindex;
     const CBlockIndex* currentBestChainLockBlockIndex;

src/llmq/quorums_instantsend.cpp

@@ -458,7 +458,10 @@ bool CInstantSendManager::ProcessTx(const CTransaction& tx, bool allowReSigning,
     for (size_t i = 0; i < tx.vin.size(); i++) {
         auto& in = tx.vin[i];
         auto& id = ids[i];
-        inputRequestIds.emplace(id);
+        {
+            LOCK(cs);
+            inputRequestIds.emplace(id);
+        }
         LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txid=%s: trying to vote on input %s with id %s. allowReSigning=%d\n", __func__,
                  tx.GetHash().ToString(), in.prevout.ToStringShort(), id.ToString(), allowReSigning);
         if (quorumSigningManager->AsyncSignIfMember(llmqType, id, tx.GetHash(), allowReSigning)) {
@@ -1119,6 +1122,7 @@ void CInstantSendManager::RemoveConflictedTx(const CTransaction& tx)
 
 void CInstantSendManager::TruncateRecoveredSigsForInputs(const llmq::CInstantSendLock& islock)
 {
+    AssertLockHeld(cs);
     auto& consensusParams = Params().GetConsensus();
 
     for (auto& in : islock.inputs) {

src/spork.cpp

@@ -186,8 +186,6 @@ bool CSporkManager::UpdateSpork(SporkId nSporkID, int64_t nValue, CConnman& conn
 {
     CSporkMessage spork = CSporkMessage(nSporkID, nValue, GetAdjustedTime());
 
-    LOCK(cs);
-
     if (!spork.Sign(sporkPrivKey)) {
         LogPrintf("CSporkManager::%s -- ERROR: signing failed for spork %d\n", __func__, nSporkID);
         return false;
@@ -201,8 +199,11 @@ bool CSporkManager::UpdateSpork(SporkId nSporkID, int64_t nValue, CConnman& conn
 
     LogPrintf("CSporkManager::%s -- signed %d %s\n", __func__, nSporkID, spork.GetHash().ToString());
 
-    mapSporksByHash[spork.GetHash()] = spork;
+    {
-    mapSporksActive[nSporkID][keyIDSigner] = spork;
+        LOCK(cs);
+        mapSporksByHash[spork.GetHash()] = spork;
+        mapSporksActive[nSporkID][keyIDSigner] = spork;
+    }
 
     spork.Relay(connman);
     return true;

src/spork.h

@@ -191,6 +191,7 @@ public:
         // we don't serialize pubkey ids because pubkeys should be
         // hardcoded or be setted with cmdline or options, should
         // not reuse pubkeys from previous dashd run
+        LOCK(cs);
         READWRITE(mapSporksByHash);
         READWRITE(mapSporksActive);
         // we don't serialize private key to prevent its leakage