validation: Keep chain tip in candidate set #33130

1. 
   nervana21 commented at 2:15 pm on August 3, 2025: contributor

   After PruneBlockIndexCandidates() is called, the candidate set must include the current chain tip or a successor of it.

   PruneBlockIndexCandidates() removes candidates with less work than the active tip. However, because the tip itself is never added to setBlockIndexCandidates via TryAddBlockIndexCandidate(), it may be absent from the set. Prior to this patch, if all lower-work candidates were pruned and the tip was not present, setBlockIndexCandidates could become empty—violating an invariant relied upon by FindMostWorkChain().

   This patch ensures that PruneBlockIndexCandidates() explicitly reinserts the current tip after pruning, preserving that invariant.

   Fixes #33129

2. DrahtBot added the label Validation on Aug 3, 2025
3. 
   DrahtBot commented at 2:15 pm on August 3, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33130.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #33127 (Adding alert for failure to prevent dead-end user crash by Ataraxia009)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

4. validation: Keep chain tip in candidate set

   After PruneBlockIndexCandidates() is called, the candidate
   set must include the current chain tip or a successor of it.
   
   PruneBlockIndexCandidates() removes candidates with less work than
   the active tip. However, because the tip itself is never added to
   setBlockIndexCandidates via TryAddBlockIndexCandidate(), it may be
   absent from the set. Prior to this patch, if all lower-work candidates
   were pruned and the tip was not present, setBlockIndexCandidates
   could become empty—violating an invariant relied upon by
   FindMostWorkChain().
   
   This patch ensures that PruneBlockIndexCandidates() explicitly
   reinserts the current tip after pruning, preserving that invariant.

   cfafc99a47
5. nervana21 force-pushed on Aug 3, 2025
6. DrahtBot added the label CI failed on Aug 3, 2025
7. 
   DrahtBot commented at 2:20 pm on August 3, 2025: contributor

   🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/47284029602 LLM reason (✨ experimental): The CI failure is caused by a trailing whitespace error detected during the lint check.

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

8. 
   nervana21 commented at 2:22 pm on August 3, 2025: contributor

   cc: @Ataraxia009
9. DrahtBot removed the label CI failed on Aug 3, 2025
10. 
    mzumsande commented at 9:59 pm on August 3, 2025: contributor

    After PruneBlockIndexCandidates() is called, the candidate set must include the current chain tip or a successor of it.

    setBlockIndexCandidates must at all times include all connectable blocks that have at least as much work as the tip, so the tip itself must at all times be in the set.

    However, because the tip itself is never added to setBlockIndexCandidates via TryAddBlockIndexCandidate(), it may be absent from the set.

    This seems wrong / makes no sense to me. No block can ever become the tip unless it’s added to setBlockIndexCandidates before. That’s what the name means, the set includes candidates for the tip.

    This patch ensures that PruneBlockIndexCandidates() explicitly reinserts the current tip after pruning, preserving that invariant.

    I don’t think that this should be done without an explanation how we could actually get into a situation where the tip is missing from the set. If it’s DB corruption, it seems better to crash instead of trying to keep a node artificially alive that will likely be out of consensus.

11. 
    nervana21 commented at 0:36 am on August 4, 2025: contributor

    Hi mzumsande, Thanks for the thorough feedback.

    You’re right, it is preferable to crash a node that is out of consensus. I believe that I did have a chain sequence construction that led to setBlockIndexCandidates without chain tip (now I’m less confident), but even so, I’m certain the chain construction would be out of consensus and therefore not worth salvaging. I wish I had considered before and I will try to consider in the future.

    Thanks again!

12. nervana21 closed this on Aug 4, 2025

    ---

Contributors
nervana21 DrahtBot mzumsande

Labels
Validation