Merge #2951: Dash LLMQ backports part 5

a85b450 Merge pull request dashpay#3399 from codablock/pr_speedups2 (Alexander Block)
136f900 cherry-pick dashpay#2833 (Alexander Block)
d942439 Merge pull request dashpay#3389 from codablock/pr_concentrated_recovery (Alexander Block)
36790d2 cherry pick dashpay#3368 (Author Alexander Block)
dac01a9 cherry-pick dashpay#2780 (Alexander Block)
39d0ed9 cherry-pick dashpay#3367 (Alexander Block)
5084bbf Allow re-signing of IS locks when performing retroactive signing (dashpay#3219) (Alexander Block)
802c006 Only track last seen time instead of first and last seen time (dashpay#3165) (Alexander Block)
479b64b Avoid propagating InstantSend related old recovered sigs (dashpay#3145) (Alexander Block)
27fa2af cherry-pick dashpay#3117 (Pasta)
cf138e0 cherry-pick dashpay#3097 (Pasta)
23b140e Introduce getbestchainlock rpc and fix llmq-is-cl-conflicts.py (dashpay#3094) (UdjinM6)

Pull request description:

  as usual each commit backports a different PR

ACKs for top commit: a85b450
  Duddino:
    utACK a85b450
  Liquid369:
    uTACK a85b450
  Fuzzbawls:
    ACK a85b450

Tree-SHA512: e9024d180888d8a6cc300ba9df74fc15929e3ade1773e5d312bd8cc93f6c9fd3898c5bf2d14672abf4faba576575c33936708e6e1dfd01a393479d264d3f2c57

Author: Fuzzbawls

src/llmq/quorums.cpp

@@ -34,7 +34,7 @@ static uint256 MakeQuorumKey(const CQuorum& q)
 {
     CHashWriter hw(SER_NETWORK, 0);
     hw << (uint8_t)q.params.type;
-    hw << q.pindexQuorum->GetBlockHash();
+    hw << q.qc.quorumHash;
     for (const auto& dmn : q.members) {
         hw << dmn->proTxHash;
     }
@@ -52,13 +52,12 @@ CQuorum::~CQuorum()
     }
 }
 
-void CQuorum::Init(const uint256& _minedBlockHash, const CBlockIndex* _pindexQuorum, const std::vector<CDeterministicMNCPtr>& _members, const std::vector<bool>& _validMembers, const CBLSPublicKey& _quorumPublicKey)
+void CQuorum::Init(const CFinalCommitment& _qc, const CBlockIndex* _pindexQuorum, const uint256& _minedBlockHash, const std::vector<CDeterministicMNCPtr>& _members)
 {
-    minedBlockHash = _minedBlockHash;
+    qc = _qc;
     pindexQuorum = _pindexQuorum;
     members = _members;
-    validMembers = _validMembers;
-    quorumPublicKey = _quorumPublicKey;
+    minedBlockHash = _minedBlockHash;
 }
 
 bool CQuorum::IsMember(const uint256& proTxHash) const
@@ -75,15 +74,15 @@ bool CQuorum::IsValidMember(const uint256& proTxHash) const
 {
     for (size_t i = 0; i < members.size(); i++) {
         if (members[i]->proTxHash == proTxHash) {
-            return validMembers[i];
+            return qc.validMembers[i];
         }
     }
     return false;
 }
 
 CBLSPublicKey CQuorum::GetPubKeyShare(size_t memberIdx) const
 {
-    if (quorumVvec == nullptr || memberIdx >= members.size() || !validMembers[memberIdx]) {
+    if (quorumVvec == nullptr || memberIdx >= members.size() || !qc.validMembers[memberIdx]) {
         return CBLSPublicKey();
     }
     auto& m = members[memberIdx];
@@ -148,7 +147,7 @@ void CQuorum::StartCachePopulatorThread(std::shared_ptr<CQuorum> _this)
     // when then later some other thread tries to get keys, it will be much faster
     _this->cachePopulatorThread = std::thread(&TraceThread<std::function<void()> >, "quorum-cachepop", [_this, t] {
         for (size_t i = 0; i < _this->members.size() && !_this->stopCachePopulatorThread && !ShutdownRequested(); i++) {
-            if (_this->validMembers[i]) {
+            if (_this->qc.validMembers[i]) {
                 _this->GetPubKeyShare(i);
             }
         }
@@ -186,7 +185,7 @@ bool CQuorumManager::BuildQuorumFromCommitment(const CFinalCommitment& qc, const
     assert(qc.quorumHash == pindexQuorum->GetBlockHash());
 
     auto members = deterministicMNManager->GetAllQuorumMembers((Consensus::LLMQType)qc.llmqType, pindexQuorum);
-    quorum->Init(minedBlockHash, pindexQuorum, members, qc.validMembers, qc.quorumPublicKey);
+    quorum->Init(qc, pindexQuorum, minedBlockHash, members);
 
     bool hasValidVvec = false;
     if (quorum->ReadContributions(evoDb)) {

src/llmq/quorums.h

@@ -35,11 +35,10 @@ class CQuorum
 
 public:
     const Consensus::LLMQParams& params;
-    uint256 minedBlockHash;
+    CFinalCommitment qc;
     const CBlockIndex* pindexQuorum;
+    uint256 minedBlockHash;
     std::vector<CDeterministicMNCPtr> members;
-    std::vector<bool> validMembers;
-    CBLSPublicKey quorumPublicKey;
 
     // These are only valid when we either participated in the DKG or fully watched it
     BLSVerificationVectorPtr quorumVvec;
@@ -55,7 +54,7 @@ class CQuorum
 public:
     CQuorum(const Consensus::LLMQParams& _params, CBLSWorker& _blsWorker) : params(_params), blsCache(_blsWorker), stopCachePopulatorThread(false) {}
     ~CQuorum();
-    void Init(const uint256& minedBlockHash, const CBlockIndex* pindexQuorum, const std::vector<CDeterministicMNCPtr>& members, const std::vector<bool>& validMembers, const CBLSPublicKey& quorumPublicKey);
+    void Init(const CFinalCommitment& _qc, const CBlockIndex* _pindexQuorum, const uint256& _minedBlockHash, const std::vector<CDeterministicMNCPtr>& _members);
 
     bool IsMember(const uint256& proTxHash) const;
     bool IsValidMember(const uint256& proTxHash) const;

src/llmq/quorums_chainlocks.cpp

@@ -23,6 +23,11 @@ static const std::string CLSIG_REQUESTID_PREFIX = "clsig";
 
 std::unique_ptr<CChainLocksHandler> chainLocksHandler{nullptr};
 
+bool CChainLockSig::IsNull() const
+{
+    return nHeight == -1 && blockHash == uint256();
+}
+
 std::string CChainLockSig::ToString() const
 {
     return strprintf("CChainLockSig(nHeight=%d, blockHash=%s)", nHeight, blockHash.ToString());
@@ -72,6 +77,12 @@ bool CChainLocksHandler::GetChainLockByHash(const uint256& hash, llmq::CChainLoc
     return true;
 }
 
+CChainLockSig CChainLocksHandler::GetBestChainLock()
+{
+    LOCK(cs);
+    return bestChainLock;
+}
+
 void CChainLocksHandler::ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv, CConnman& connman)
 {
     if (!sporkManager.IsSporkActive(SPORK_23_CHAINLOCKS_ENFORCEMENT)) {
@@ -101,7 +112,7 @@ void CChainLocksHandler::ProcessNewChainLock(NodeId from, const llmq::CChainLock
             return;
         }
 
-        if (bestChainLock.nHeight != -1 && clsig.nHeight <= bestChainLock.nHeight) {
+        if (!bestChainLock.IsNull() && clsig.nHeight <= bestChainLock.nHeight) {
             // no need to process/relay older CLSIGs
             return;
         }
@@ -133,7 +144,7 @@ void CChainLocksHandler::ProcessNewChainLock(NodeId from, const llmq::CChainLock
         bestChainLock = clsig;
 
         CInv inv(MSG_CLSIG, hash);
-        g_connman->RelayInv(inv);
+        g_connman->RelayInv(inv, LLMQS_PROTO_VERSION);
 
         auto blockIt = mapBlockIndex.find(clsig.blockHash);
         if (blockIt == mapBlockIndex.end()) {

src/llmq/quorums_chainlocks.h

@@ -34,6 +34,8 @@ class CChainLockSig
         READWRITE(obj.blockHash);
         READWRITE(obj.sig);
     }
+
+    bool IsNull() const;
     std::string ToString() const;
 };
 
@@ -70,6 +72,7 @@ class CChainLocksHandler : public CRecoveredSigsListener
 public:
     bool AlreadyHave(const CInv& inv);
     bool GetChainLockByHash(const uint256& hash, CChainLockSig& ret);
+    CChainLockSig GetBestChainLock();
 
     void ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv, CConnman& connman);
     void ProcessNewChainLock(NodeId from, const CChainLockSig& clsig, const uint256& hash);

src/llmq/quorums_connections.cpp

@@ -124,9 +124,8 @@ void EnsureLatestQuorumConnections(Consensus::LLMQType llmqType, const CBlockInd
             continue;
         }
 
-        if (!connman->hasQuorumNodes(llmqType, quorum->pindexQuorum->GetBlockHash())) {
-            EnsureQuorumConnections(llmqType, quorum->pindexQuorum, myProTxHash);
-        }
+        EnsureQuorumConnections(llmqType, quorum->pindexQuorum, myProTxHash);
+
         connmanQuorumsToDelete.erase(quorum->pindexQuorum->GetBlockHash());
     }
 

src/llmq/quorums_signing.cpp

@@ -246,7 +246,7 @@ void CRecoveredSigsDb::WriteRecoveredSig(const llmq::CRecoveredSig& recSig)
     }
 }
 
-void CRecoveredSigsDb::RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType llmqType, const uint256& id, bool deleteTimeKey)
+void CRecoveredSigsDb::RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType llmqType, const uint256& id, bool deleteHashKey, bool deleteTimeKey)
 {
     AssertLockHeld(cs);
 
@@ -263,7 +263,9 @@ void CRecoveredSigsDb::RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType l
     auto k4 = std::make_tuple(std::string("rs_s"), signHash);
     batch.Erase(k1);
     batch.Erase(k2);
-    batch.Erase(k3);
+    if (deleteHashKey) {
+        batch.Erase(k3);
+    }
     batch.Erase(k4);
 
     if (deleteTimeKey) {
@@ -279,14 +281,27 @@ void CRecoveredSigsDb::RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType l
 
     hasSigForIdCache.erase(std::make_pair((Consensus::LLMQType)recSig.llmqType, recSig.id));
     hasSigForSessionCache.erase(signHash);
-    hasSigForHashCache.erase(recSig.GetHash());
+    if (deleteHashKey) {
+        hasSigForHashCache.erase(recSig.GetHash());
+    }
 }
 
+// Completely remove any traces of the recovered sig
 void CRecoveredSigsDb::RemoveRecoveredSig(Consensus::LLMQType llmqType, const uint256& id)
 {
     LOCK(cs);
     CDBBatch batch(CLIENT_VERSION | ADDRV2_FORMAT);
-    RemoveRecoveredSig(batch, llmqType, id, true);
+    RemoveRecoveredSig(batch, llmqType, id, true, true);
+    db.WriteBatch(batch);
+}
+
+// Remove the recovered sig itself and all keys required to get from id -> recSig
+// This will leave the byHash key in-place so that HasRecoveredSigForHash still returns true
+void CRecoveredSigsDb::TruncateRecoveredSig(Consensus::LLMQType llmqType, const uint256& id)
+{
+    LOCK(cs);
+    CDBBatch batch(CLIENT_VERSION | ADDRV2_FORMAT);
+    RemoveRecoveredSig(batch, llmqType, id, false, false);
     db.WriteBatch(batch);
 }
 
@@ -326,7 +341,7 @@ void CRecoveredSigsDb::CleanupOldRecoveredSigs(int64_t maxAge)
     {
         LOCK(cs);
         for (auto& e : toDelete) {
-            RemoveRecoveredSig(batch, e.first, e.second, false);
+            RemoveRecoveredSig(batch, e.first, e.second, true, false);
 
             if (batch.SizeEstimate() >= (1 << 24)) {
                 db.WriteBatch(batch);
@@ -417,8 +432,10 @@ CSigningManager::CSigningManager(CDBWrapper& llmqDb, bool fMemory) : db(llmqDb)
 
 bool CSigningManager::AlreadyHave(const CInv& inv)
 {
-    LOCK(cs);
-    return inv.type == MSG_QUORUM_RECOVERED_SIG && db.HasRecoveredSigForHash(inv.hash);
+    if (inv.type != MSG_QUORUM_RECOVERED_SIG) {
+        return false;
+    }
+    return db.HasRecoveredSigForHash(inv.hash);
 }
 
 bool CSigningManager::GetRecoveredSigForGetData(const uint256& hash, CRecoveredSig& ret)
@@ -459,7 +476,8 @@ void CSigningManager::ProcessMessageRecoveredSig(CNode* pfrom, const CRecoveredS
         return;
     }
 
-    LogPrint(BCLog::LLMQ, "CSigningManager::%s -- signHash=%s, node=%d\n", __func__, llmq::utils::BuildSignHash(recoveredSig).ToString(), pfrom->GetId());
+    LogPrint(BCLog::LLMQ, "CSigningManager::%s -- signHash=%s, id=%s, msgHash=%s, node=%d\n", __func__,
+        llmq::utils::BuildSignHash(recoveredSig).ToString(), recoveredSig.id.ToString(), recoveredSig.msgHash.ToString(), pfrom->GetId());
 
     LOCK(cs);
     pendingRecoveredSigs[pfrom->GetId()].emplace_back(recoveredSig);
@@ -482,7 +500,7 @@ bool CSigningManager::PreVerifyRecoveredSig(NodeId nodeId, const CRecoveredSig&
             recoveredSig.quorumHash.ToString(), nodeId);
         return false;
     }
-    if (!llmq::utils::IsQuorumActive(llmqType, quorum->pindexQuorum->GetBlockHash())) {
+    if (!llmq::utils::IsQuorumActive(llmqType, quorum->qc.quorumHash)) {
         return false;
     }
 
@@ -537,7 +555,7 @@ void CSigningManager::CollectPendingRecoveredSigsToVerify(
                     it = v.erase(it);
                     continue;
                 }
-                if (!llmq::utils::IsQuorumActive(llmqType, quorum->pindexQuorum->GetBlockHash())) {
+                if (!llmq::utils::IsQuorumActive(llmqType, quorum->qc.quorumHash)) {
                     LogPrint(BCLog::LLMQ, "CSigningManager::%s -- quorum %s not active anymore, node=%d\n", __func__,
                         recSig.quorumHash.ToString(), nodeId);
                     it = v.erase(it);
@@ -579,7 +597,7 @@ bool CSigningManager::ProcessPendingRecoveredSigs(CConnman& connman)
             }
 
             const auto& quorum = quorums.at(std::make_pair((Consensus::LLMQType)recSig.llmqType, recSig.quorumHash));
-            batchVerifier.PushMessage(nodeId, recSig.GetHash(), llmq::utils::BuildSignHash(recSig), recSig.sig.Get(), quorum->quorumPublicKey);
+            batchVerifier.PushMessage(nodeId, recSig.GetHash(), llmq::utils::BuildSignHash(recSig), recSig.sig.Get(), quorum->qc.quorumPublicKey);
             verifyCount++;
         }
     }
@@ -624,6 +642,11 @@ void CSigningManager::ProcessRecoveredSig(NodeId nodeId, const CRecoveredSig& re
         LOCK(cs_main);
         connman.RemoveAskFor(recoveredSig.GetHash(), MSG_QUORUM_RECOVERED_SIG);
     }
+
+    if (db.HasRecoveredSigForHash(recoveredSig.GetHash())) {
+        return;
+    }
+
     std::vector<CRecoveredSigsListener*> listeners;
     {
         LOCK(cs);
@@ -661,7 +684,7 @@ void CSigningManager::ProcessRecoveredSig(NodeId nodeId, const CRecoveredSig& re
 
     CInv inv(MSG_QUORUM_RECOVERED_SIG, recoveredSig.GetHash());
     g_connman->ForEachNode([&](CNode* pnode) {
-        if (pnode->m_wants_recsigs) {
+        if (pnode->nVersion >= LLMQS_PROTO_VERSION && pnode->m_wants_recsigs && pnode->CanRelay()) {
             pnode->PushInventory(inv);
         }
     });
@@ -670,9 +693,9 @@ void CSigningManager::ProcessRecoveredSig(NodeId nodeId, const CRecoveredSig& re
     }
 }
 
-void CSigningManager::RemoveRecoveredSig(Consensus::LLMQType llmqType, const uint256& id)
+void CSigningManager::TruncateRecoveredSig(Consensus::LLMQType llmqType, const uint256& id)
 {
-    db.RemoveRecoveredSig(llmqType, id);
+    db.TruncateRecoveredSig(llmqType, id);
 }
 
 void CSigningManager::Cleanup()
@@ -703,7 +726,7 @@ void CSigningManager::UnregisterRecoveredSigsListener(CRecoveredSigsListener* l)
     recoveredSigsListeners.erase(itRem, recoveredSigsListeners.end());
 }
 
-bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash)
+bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash, bool allowReSign)
 {
     auto& params = Params().GetConsensus().llmqs.at(llmqType);
 
@@ -714,24 +737,31 @@ bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, const uint
     {
         LOCK(cs);
 
-        if (db.HasVotedOnId(llmqType, id)) {
+        bool hasVoted = db.HasVotedOnId(llmqType, id);
+        if (hasVoted) {
             uint256 prevMsgHash;
             db.GetVoteForId(llmqType, id, prevMsgHash);
             if (msgHash != prevMsgHash) {
                 LogPrintf("CSigningManager::%s -- already voted for id=%s and msgHash=%s. Not voting on conflicting msgHash=%s\n", __func__,
                     id.ToString(), prevMsgHash.ToString(), msgHash.ToString());
+                return false;
+            } else if (allowReSign) {
+                LogPrint(BCLog::LLMQ, "CSigningManager::%s -- already voted for id=%s and msgHash=%s. Resigning!\n", __func__,
+                    id.ToString(), prevMsgHash.ToString());
             } else {
                 LogPrint(BCLog::LLMQ, "CSigningManager::%s -- already voted for id=%s and msgHash=%s. Not voting again.\n", __func__,
                     id.ToString(), prevMsgHash.ToString());
+                return false;
             }
-            return false;
         }
 
         if (db.HasRecoveredSigForId(llmqType, id)) {
             // no need to sign it if we already have a recovered sig
             return true;
         }
-        db.WriteVoteForId(llmqType, id, msgHash);
+        if (!hasVoted) {
+            db.WriteVoteForId(llmqType, id, msgHash);
+        }
     }
 
     int tipHeight;
@@ -752,10 +782,13 @@ bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, const uint
     }
 
     if (!quorum->IsValidMember(activeMasternodeManager->GetProTx())) {
-        // LogPrint(BCLog::LLMQ, "CSigningManager::%s -- we're not a valid member of quorum %s\n", __func__, quorum->quorumHash.ToString());
         return false;
     }
 
+    if (allowReSign) {
+        // make us re-announce all known shares (other nodes might have run into a timeout)
+        quorumSigSharesManager->ForceReAnnouncement(quorum, llmqType, id, msgHash);
+    }
     quorumSigSharesManager->AsyncSign(quorum, id, msgHash);
 
     return true;
@@ -832,7 +865,7 @@ CQuorumCPtr CSigningManager::SelectQuorumForSigning(Consensus::LLMQType llmqType
     for (size_t i = 0; i < quorums.size(); i++) {
         CHashWriter h(SER_NETWORK, 0);
         h << (uint8_t)llmqType;
-        h << quorums[i]->pindexQuorum->GetBlockHash();
+        h << quorums[i]->qc.quorumHash;
         h << selectionHash;
         scores.emplace_back(h.GetHash(), i);
     }
@@ -849,8 +882,8 @@ bool CSigningManager::VerifyRecoveredSig(Consensus::LLMQType llmqType, int signe
         return false;
     }
 
-    uint256 signHash = llmq::utils::BuildSignHash(llmqParams.type, quorum->pindexQuorum->GetBlockHash(), id, msgHash);
-    return sig.VerifyInsecure(quorum->quorumPublicKey, signHash);
+    uint256 signHash = llmq::utils::BuildSignHash(llmqParams.type, quorum->qc.quorumHash, id, msgHash);
+    return sig.VerifyInsecure(quorum->qc.quorumPublicKey, signHash);
 }
 
 } // namespace llmq