From 77b509a3d2a3e355b5d2a1d3b94e42dd0ff46fb6 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Thu, 8 Aug 2019 07:40:34 +0200 Subject: [PATCH 1/3] Split ProcessPendingInstantSendLocks into two methods --- src/llmq/quorums_instantsend.cpp | 11 ++++++++--- src/llmq/quorums_instantsend.h | 1 + 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 1f12ea257697c..4c92c239e69df 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -715,8 +715,6 @@ bool CInstantSendManager::PreVerifyInstantSendLock(NodeId nodeId, const llmq::CI bool CInstantSendManager::ProcessPendingInstantSendLocks() { - auto llmqType = Params().GetConsensus().llmqForInstantSend; - decltype(pendingInstantSendLocks) pend; { @@ -738,6 +736,13 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks() tipHeight = chainActive.Height(); } + return ProcessPendingInstantSendLocks(tipHeight, pend); +} + +bool CInstantSendManager::ProcessPendingInstantSendLocks(int signHeight, const std::unordered_map>& pend) +{ + auto llmqType = Params().GetConsensus().llmqForInstantSend; + CBLSBatchVerifier batchVerifier(false, true, 8); std::unordered_map> recSigs; @@ -762,7 +767,7 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks() continue; } - auto quorum = quorumSigningManager->SelectQuorumForSigning(llmqType, tipHeight, id); + auto quorum = quorumSigningManager->SelectQuorumForSigning(llmqType, signHeight, id); if (!quorum) { // should not happen, but if one fails to select, all others will also fail to select return false; diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index 1a7521c2dd576..2d1581a575dab 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -138,6 +138,7 @@ class CInstantSendManager : public CRecoveredSigsListener void ProcessMessageInstantSendLock(CNode* pfrom, const CInstantSendLock& islock, CConnman& connman); bool PreVerifyInstantSendLock(NodeId nodeId, const CInstantSendLock& islock, bool& retBan); bool ProcessPendingInstantSendLocks(); + bool ProcessPendingInstantSendLocks(int signHeight, const std::unordered_map>& pend); void ProcessInstantSendLock(NodeId from, const uint256& hash, const CInstantSendLock& islock); void UpdateWalletTransaction(const CTransactionRef& tx, const CInstantSendLock& islock); From 770126a365017a49beb9619be2089fbacc47286d Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Thu, 8 Aug 2019 07:51:58 +0200 Subject: [PATCH 2/3] Split SelectQuorumForSigning into SelectQuorumForSigning and GetActiveQuorumSet --- src/llmq/quorums_signing.cpp | 11 ++++++++--- src/llmq/quorums_signing.h | 1 + 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/llmq/quorums_signing.cpp b/src/llmq/quorums_signing.cpp index 708872f4a67c1..e0996df66a04d 100644 --- a/src/llmq/quorums_signing.cpp +++ b/src/llmq/quorums_signing.cpp @@ -815,7 +815,7 @@ bool CSigningManager::GetVoteForId(Consensus::LLMQType llmqType, const uint256& return db.GetVoteForId(llmqType, id, msgHashRet); } -CQuorumCPtr CSigningManager::SelectQuorumForSigning(Consensus::LLMQType llmqType, int signHeight, const uint256& selectionHash) +std::vector CSigningManager::GetActiveQuorumSet(Consensus::LLMQType llmqType, int signHeight) { auto& llmqParams = Params().GetConsensus().llmqs.at(llmqType); size_t poolSize = (size_t)llmqParams.signingActiveQuorumCount; @@ -825,12 +825,17 @@ CQuorumCPtr CSigningManager::SelectQuorumForSigning(Consensus::LLMQType llmqType LOCK(cs_main); int startBlockHeight = signHeight - SIGN_HEIGHT_OFFSET; if (startBlockHeight > chainActive.Height()) { - return nullptr; + return {}; } pindexStart = chainActive[startBlockHeight]; } - auto quorums = quorumManager->ScanQuorums(llmqType, pindexStart, poolSize); + return quorumManager->ScanQuorums(llmqType, pindexStart, poolSize); +} + +CQuorumCPtr CSigningManager::SelectQuorumForSigning(Consensus::LLMQType llmqType, int signHeight, const uint256& selectionHash) +{ + auto quorums = GetActiveQuorumSet(llmqType, signHeight); if (quorums.empty()) { return nullptr; } diff --git a/src/llmq/quorums_signing.h b/src/llmq/quorums_signing.h index 0eec5c3934907..294d3190129ba 100644 --- a/src/llmq/quorums_signing.h +++ b/src/llmq/quorums_signing.h @@ -171,6 +171,7 @@ class CSigningManager bool HasVotedOnId(Consensus::LLMQType llmqType, const uint256& id); bool GetVoteForId(Consensus::LLMQType llmqType, const uint256& id, uint256& msgHashRet); + std::vector GetActiveQuorumSet(Consensus::LLMQType llmqType, int signHeight); CQuorumCPtr SelectQuorumForSigning(Consensus::LLMQType llmqType, int signHeight, const uint256& selectionHash); // Verifies a recovered sig that was signed while the chain tip was at signedAtTip From b1362d181dfb1378ca6e3ab130c88ef261dca807 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Thu, 8 Aug 2019 10:11:49 +0200 Subject: [PATCH 3/3] Implement retrying of IS lock verification when the LLMQ active set rotates --- src/llmq/quorums_instantsend.cpp | 44 ++++++++++++++++++++++++++++---- src/llmq/quorums_instantsend.h | 2 +- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 4c92c239e69df..670cab70ab184 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -736,10 +736,41 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks() tipHeight = chainActive.Height(); } - return ProcessPendingInstantSendLocks(tipHeight, pend); + auto llmqType = Params().GetConsensus().llmqForInstantSend; + + // Every time a new quorum enters the active set, an older one is removed. This means that between two blocks, the + // active set can be different, leading to different selection of the signing quorum. When we detect such rotation + // of the active set, we must re-check invalid sigs against the previous active set and only ban nodes when this also + // fails. + auto quorums1 = quorumSigningManager->GetActiveQuorumSet(llmqType, tipHeight); + auto quorums2 = quorumSigningManager->GetActiveQuorumSet(llmqType, tipHeight - 1); + bool quorumsRotated = quorums1 != quorums2; + + if (quorumsRotated) { + // first check against the current active set and don't ban + auto badISLocks = ProcessPendingInstantSendLocks(tipHeight, pend, false); + if (!badISLocks.empty()) { + LogPrintf("CInstantSendManager::%s -- detected LLMQ active set rotation, redoing verification on old active set\n", __func__); + + // filter out valid IS locks from "pend" + for (auto it = pend.begin(); it != pend.end(); ) { + if (!badISLocks.count(it->first)) { + it = pend.erase(it); + } else { + ++it; + } + } + // now check against the previous active set and perform banning if this fails + ProcessPendingInstantSendLocks(tipHeight - 1, pend, true); + } + } else { + ProcessPendingInstantSendLocks(tipHeight, pend, true); + } + + return true; } -bool CInstantSendManager::ProcessPendingInstantSendLocks(int signHeight, const std::unordered_map>& pend) +std::unordered_set CInstantSendManager::ProcessPendingInstantSendLocks(int signHeight, const std::unordered_map>& pend, bool ban) { auto llmqType = Params().GetConsensus().llmqForInstantSend; @@ -770,7 +801,7 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks(int signHeight, const s auto quorum = quorumSigningManager->SelectQuorumForSigning(llmqType, signHeight, id); if (!quorum) { // should not happen, but if one fails to select, all others will also fail to select - return false; + return {}; } uint256 signHash = CLLMQUtils::BuildSignHash(llmqType, quorum->qc.quorumHash, id, islock.txid); batchVerifier.PushMessage(nodeId, hash, signHash, islock.sig.Get(), quorum->qc.quorumPublicKey); @@ -793,7 +824,9 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks(int signHeight, const s batchVerifier.Verify(); - if (!batchVerifier.badSources.empty()) { + std::unordered_set badISLocks; + + if (ban && !batchVerifier.badSources.empty()) { LOCK(cs_main); for (auto& nodeId : batchVerifier.badSources) { // Let's not be too harsh, as the peer might simply be unlucky and might have sent us an old lock which @@ -809,6 +842,7 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks(int signHeight, const s if (batchVerifier.badMessages.count(hash)) { LogPrintf("CInstantSendManager::%s -- txid=%s, islock=%s: invalid sig in islock, peer=%d\n", __func__, islock.txid.ToString(), hash.ToString(), nodeId); + badISLocks.emplace(hash); continue; } @@ -829,7 +863,7 @@ bool CInstantSendManager::ProcessPendingInstantSendLocks(int signHeight, const s } } - return true; + return badISLocks; } void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& hash, const CInstantSendLock& islock) diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index 2d1581a575dab..0ee460d8d6a73 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -138,7 +138,7 @@ class CInstantSendManager : public CRecoveredSigsListener void ProcessMessageInstantSendLock(CNode* pfrom, const CInstantSendLock& islock, CConnman& connman); bool PreVerifyInstantSendLock(NodeId nodeId, const CInstantSendLock& islock, bool& retBan); bool ProcessPendingInstantSendLocks(); - bool ProcessPendingInstantSendLocks(int signHeight, const std::unordered_map>& pend); + std::unordered_set ProcessPendingInstantSendLocks(int signHeight, const std::unordered_map>& pend, bool ban); void ProcessInstantSendLock(NodeId from, const uint256& hash, const CInstantSendLock& islock); void UpdateWalletTransaction(const CTransactionRef& tx, const CInstantSendLock& islock);