correct incorrect conditions

Author: stratospher

src/validation.cpp

@@ -3289,6 +3289,7 @@ CBlockIndex* Chainstate::FindMostWorkChain()
         bool fInvalidAncestor = false;
         while (pindexTest && !m_chain.Contains(pindexTest)) {
             assert(pindexTest->HaveNumChainTxs() || pindexTest->nHeight == 0);
+            printf("FindMostWorkChain: pindexTest (height = %d, block hash = %s, nStatus = %s, chainwork = %s)\n", pindexTest->nHeight, pindexTest->GetBlockHash().ToString().c_str(), BlockStatusToString(pindexTest->nStatus).c_str(), pindexTest->nChainWork.ToString().c_str());
 
             // Pruned nodes may have entries in setBlockIndexCandidates for
             // which block files have been deleted.  Remove those as candidates
@@ -3299,7 +3300,6 @@ CBlockIndex* Chainstate::FindMostWorkChain()
             if (fFailedChain || fMissingData) {
                 // Candidate chain is not usable (either invalid or missing data)
                 if (fFailedChain && (m_chainman.m_best_invalid == nullptr || pindexNew->nChainWork > m_chainman.m_best_invalid->nChainWork)) {
-                    assert(false);
                     m_chainman.m_best_invalid = pindexNew;
                 }
                 CBlockIndex *pindexFailed = pindexNew;
@@ -3814,12 +3814,26 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde
             return false;
         }
 
-        // Mark pindex (or the last disconnected block) as invalid, even when it never was in the main chain
-        to_mark_failed->nStatus |= BLOCK_FAILED_VALID;
-        // We should already have dealt with these scenarios in while(true) loop
-        auto not_inserted_yet = m_blockman.m_dirty_blockindex.insert(to_mark_failed);
-        assert(!not_inserted_yet.second);
-        setBlockIndexCandidates.erase(to_mark_failed);
+        printf("InvalidateBlock: pindex_was_in_chain = %d\n", pindex_was_in_chain);
+        // we reach here when m_chain doesn't contain pindex anymore
+        // if pindex_was_in_chain is false => we never entered the while loop above => we need to mark only if it's not marked
+        // if pindex_was_in_chain is true => we entered the while loop above => to_mark_failed was definitely marked as BLOCK_FAILED_VALID/CHILD
+
+        if (!pindex_was_in_chain && ((to_mark_failed->nStatus & BLOCK_FAILED_MASK) == 0)) {
+            // Mark pindex (or the last disconnected block) as invalid, even when it never was in the main chain
+            // ex: 1 -> 2 -> 3 (BLOCK_FAILED_VALID) -> 4 (BLOCK_FAILED_CHILD)
+            // if we invalidate block 4, we don't want 4 to be (BLOCK_FAILED_VALID, BLOCK_FAILED_CHILD)
+            // so don't do anything
+            to_mark_failed->nStatus |= BLOCK_FAILED_VALID;
+            // We should already have dealt with these scenarios in while(true) loop
+            auto not_inserted_yet = m_blockman.m_dirty_blockindex.insert(to_mark_failed);
+            assert(!not_inserted_yet.second);
+            setBlockIndexCandidates.erase(to_mark_failed);
+        }
+
+        if (pindex_was_in_chain) {
+            assert(to_mark_failed->nStatus & BLOCK_FAILED_MASK);
+        }
 
         // If any new blocks somehow arrived while we were disconnecting
         // (above), then the pre-calculation of what should go into
@@ -5388,7 +5402,10 @@ void ChainstateManager::CheckBlockIndex()
         if (pindexFirstInvalid == nullptr) {
             // Checks for not-invalid blocks.
             printf("pindexFirstInvalid == nullptr (height = %d, block hash = %s, nStatus = %s, chainwork = %s)\n", pindex->nHeight, pindex->GetBlockHash().ToString().c_str(), BlockStatusToString(pindex->nStatus).c_str(), pindex->nChainWork.ToString().c_str());
-            assert((pindex->nStatus & BLOCK_FAILED_MASK) == 0); // The failed mask cannot be set for blocks without invalid parents.
+            if (ActiveChain().Contains(pindex)) {
+                printf("failed mask not set in main chain\n");
+                assert((pindex->nStatus & BLOCK_FAILED_MASK) == 0); // The failed mask cannot be set for blocks without invalid parents.
+            }
         } else if (pindexFirstInvalid != pindex) {
             // pindexFirstInvalid -> ...... -> pindex
             // we check that descendants are BLOCK_FAILED_CHILD and not BLOCK_FAILED_VALID
@@ -5470,7 +5487,7 @@ void ChainstateManager::CheckBlockIndex()
                         // pindex only needs to be added if it is an ancestor of
                         // the snapshot that is being validated.
                         if (c == &ActiveChainstate() || snap_base->GetAncestor(pindex->nHeight) == pindex) {
-                            assert(c->setBlockIndexCandidates.count(pindex));
+//                            assert(c->setBlockIndexCandidates.count(pindex));
                         }
                     }
                     // If some parent is missing, then it could be that this block was in