validation: Prevent duplicate logging and looping in invalid block handling #34254

1. 
   mzumsande commented at 4:59 pm on January 11, 2026: contributor

   InvalidChainFound() is called from 3 spots:

   1. from InvalidBlockFound()
   2. from ActivateBestChain(), with the highest connectable block of the chain as an arg (after having called InvalidBlockFound() already for the block that failed in ConnectTip())
   3. from InvalidateBlock() (rpc-only)

   Most of the logic in InvalidChainFound() is not required for 2) and 3): In ActivateBestChain(), the previous call to InvalidChainFound() before via InvalidBlockFound did the flag updates, best header calculation and logging, so only m_best_invalid could need additional updating. In InvalidateBlock() (which doesn’t call InvalidBlockFound()), we do most of the accounting manually.

   Therefore move of all logic but the m_best_invalid update into InvalidBlockFound(), avoiding duplicate logging and unnecessary work (the loops over the block index in SetBlockFailureFlags() and RecalculateBestHeader() were done twice before).

   The second commit renames InvalidChainFound to UpdateBestInvalid to adjust to its remaining role.

   This addresses past discussions about repeated work in InvalidateBlock() (https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2135717500) and ActivateBestChain() (https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2607189790)

2. validation: Move most logic from InvalidChainFound into InvalidBlockFound

   InvalidChainFound is called from 3 spots:
   1. from InvalidBlockFound
   2. from ActivateBestChain, with the highest connectable block of the chain
   as an arg (after having called InvalidBlockFound already for the block that
   actually failed)
   3. from InvalidateBlock
   
   Most of the logic in InvalidChainFound is not required for 2) and 3):
   In ActivateBestChain, we already called InvalidChainFound before via
   InvalidBlockFound, so only m_best_invalid needs updating.
   In InvalidateBlock (which doesn't call InvalidBlockFound), we do most
   of the accounting manually.
   Therefore move of all logic but the m_best_invalid update into
   InvalidBlockFound, avoiding duplicate logging and unnecessary work
   (the loops over the block index in SetBlockFailureFlags and RecalculateBestHeader
   were done twice before).

   c816490e82
3. validation: rename InvalidChainFound to UpdateBestInvalid

   It's a more descriptive name after most of the logic was moved
   into InvalidBlockFound.

   73e0bab7ad
4. DrahtBot added the label Validation on Jan 11, 2026
5. 
   DrahtBot commented at 4:59 pm on January 11, 2026: 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/34254.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	bensig

   If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

6. 
   bensig commented at 9:48 pm on January 12, 2026: contributor

   ACK 73e0bab7adb0b16b2ac4d13c9d601a04b7184749

Contributors
mzumsande DrahtBot bensig

Labels
Validation