When we call reconsiderblock for some block, Chainstate::ResetBlockFailureFlags puts the descendants of that block into setBlockIndexCandidates (if they meet the criteria, i.e. have more work than the tip etc.), but never put any ancestors into the set even though we do clear their failure flags.
I think that this is wrong, because setBlockIndexCandidates should always contain all eligible indexes that have at least as much work as the current tip, which can include ancestors of the reconsidered block. This is being checked by CheckBlockIndex(), which could fail if it was invoked after ActivateBestChain connects a block and releases cs_main:
0diff --git a/src/validation.cpp b/src/validation.cpp
1index 7b04bd9a5b..ff0c3c9f58 100644
2--- a/src/validation.cpp
3+++ b/src/validation.cpp
4@@ -3551,6 +3551,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr<
5 }
6 }
7 // When we reach this point, we switched to a new tip (stored in pindexNewTip).
8+ m_chainman.CheckBlockIndex();
9
10 if (exited_ibd) {
11 // If a background chainstate is in use, we may need to rebalance our
makes rpc_invalidateblock.py fail on master.
Even though we don’t currently have a CheckBlockIndex() in that place, after cs_main is released other threads could invoke it, which is happening in the rare failures of #16444 where an invalid header received from another peer could trigger a CheckBlockIndex() call that would fail.
Fix this by adding eligible ancestors to setBlockIndexCandidates in Chainstate::ResetBlockFailureFlags (also simplifying that function a bit).
Fixes #16444
