Implement retrying of IS lock verification when the LLMQ active set r…

…otates
dashpay · Aug 8, 2019 · b1362d1 · b1362d1
1 parent 770126a
commit b1362d1
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 6 deletions.
diff --git 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<uint256, std::pair<NodeId, CInstantSendLock>>& pend)
+std::unordered_set<uint256> CInstantSendManager::ProcessPendingInstantSendLocks(int signHeight, const std::unordered_map<uint256, std::pair<NodeId, CInstantSendLock>>& 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<uint256> 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
@@ -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<uint256, std::pair<NodeId, CInstantSendLock>>& pend);
+ std::unordered_set<uint256> ProcessPendingInstantSendLocks(int signHeight, const std::unordered_map<uint256, std::pair<NodeId, CInstantSendLock>>& pend, bool ban);
  void ProcessInstantSendLock(NodeId from, const uint256& hash, const CInstantSendLock& islock);
  void UpdateWalletTransaction(const CTransactionRef& tx, const CInstantSendLock& islock);