From b1d249d1024a1f9cbdacffc87718220122554e6b Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Sat, 11 Nov 2023 13:14:26 +0300
Subject: [PATCH] fix: avoid some crashes on `invalidateblock` (#5683)

## Issue being fixed or feature implemented
```
Assertion failure:
  assertion: quorum != nullptr
  file: quorums.cpp, line: 547
  function: ScanQuorums
```

## What was done?
Hold cs_main while scanning to make sure tip doesn't move. Happened in
`ProcessPendingInstantSendLocks()` only for me but I thought that it
would probably make sense to apply the same fix in other places too.

## How Has This Been Tested?
run `invalidateblock` for a deep enough height (100s of blocks)

## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
---
 src/llmq/instantsend.cpp | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/src/llmq/instantsend.cpp b/src/llmq/instantsend.cpp
index 9f00f54c13..093a9fd413 100644
--- a/src/llmq/instantsend.cpp
+++ b/src/llmq/instantsend.cpp
@@ -454,6 +454,14 @@ void CInstantSendDb::RemoveAndArchiveInstantSendLock(const gsl::not_null<CInstan
 
 ////////////////
 
+std::optional<Consensus::LLMQType> GetInstantSendLLMQTypeAtTip(const CQuorumManager& qman, const CChainState& chainstate)
+{
+    LOCK(cs_main);
+    const CBlockIndex* tip = chainstate.m_chain.Tip();
+    if (tip == nullptr) return std::nullopt;
+    return std::make_optional(utils::GetInstantSendLLMQType(qman, tip));
+}
+
 void CInstantSendManager::Start()
 {
     // can't start new thread if we have one running already
@@ -511,7 +519,9 @@ void CInstantSendManager::ProcessTx(const CTransaction& tx, bool fRetroactive, c
     // block after we retroactively locked all transactions.
     if (!IsInstantSendMempoolSigningEnabled() && !fRetroactive) return;
 
-    if (!TrySignInputLocks(tx, fRetroactive, utils::GetInstantSendLLMQType(qman, WITH_LOCK(cs_main, return m_chainstate.m_chain.Tip())), params)) {
+    if (auto llmqType_opt{GetInstantSendLLMQTypeAtTip(qman, m_chainstate)}; !llmqType_opt.has_value()) {
+        return;
+    } else if (!TrySignInputLocks(tx, fRetroactive, llmqType_opt.value(), params)) {
         return;
     }
 
@@ -685,7 +695,9 @@ void CInstantSendManager::HandleNewInputLockRecoveredSig(const CRecoveredSig& re
 
 void CInstantSendManager::TrySignInstantSendLock(const CTransaction& tx)
 {
-    const auto llmqType = utils::GetInstantSendLLMQType(qman, WITH_LOCK(cs_main, return m_chainstate.m_chain.Tip()));
+    const auto llmqType_opt{GetInstantSendLLMQTypeAtTip(qman, m_chainstate)};
+    if (!llmqType_opt.has_value()) return;
+    const auto llmqType = llmqType_opt.value();
 
     for (const auto& in : tx.vin) {
         auto id = ::SerializeHash(std::make_pair(INPUTLOCK_REQUESTID_PREFIX, in.prevout));
@@ -855,9 +867,10 @@ bool CInstantSendLock::TriviallyValid() const
 
 bool CInstantSendManager::ProcessPendingInstantSendLocks()
 {
-    const CBlockIndex* pBlockIndexTip = WITH_LOCK(cs_main, return m_chainstate.m_chain.Tip());
-    if (pBlockIndexTip && utils::GetInstantSendLLMQType(qman, pBlockIndexTip) == Params().GetConsensus().llmqTypeDIP0024InstantSend) {
-        // Don't short circuit. Try to process both deterministic and not deterministic islocks independable
+    if (auto llmqType_opt{GetInstantSendLLMQTypeAtTip(qman, m_chainstate)}; !llmqType_opt.has_value()) {
+        return true; // not an error
+    } else if (llmqType_opt.value() == Params().GetConsensus().llmqTypeDIP0024InstantSend) {
+         // Don't short circuit. Try to process both deterministic and not deterministic islocks independable
         bool deterministicRes = ProcessPendingInstantSendLocks(true);
         bool nondeterministicRes = ProcessPendingInstantSendLocks(false);
         return deterministicRes && nondeterministicRes;