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

full analysis

https://github.com/bitcoin/bitcoin/pull/34254 · mzumsande · +64/-30 in 6 files, 3 commits · labels: Validation

Goal

  • Prevent redundant block index traversals and duplicate log entries when handling invalid blocks
  • Clean up and clarify validation control flow during invalid block processing

Refactors invalid block handling by moving flag updates, header recalculations, and logging from InvalidChainFound into InvalidBlockFound. This avoids duplicate traversals over the block index and duplicate log entries when ActivateBestChain processes an invalid block, renaming the residual function to UpdateBestInvalid.

Problem: When ActivateBestChain encounters an invalid chain, it calls InvalidBlockFound (which called InvalidChainFound) and then immediately calls InvalidChainFound again on the highest connectable block. This resulted in redundant SetBlockFailureFlags and RecalculateBestHeader loops across the block index as well as duplicated log lines.

Category: Validation (#25 of 48)

P3 · cleanup

  • P3 because it eliminates redundant block index iterations and repeated log lines during block validation
  • Performance and log cleanups only trigger during rare invalid block events or manual invalidations
  • Helps untangle historical validation duplication and clarifies control flow

P3 because it resolves redundant block index iterations and eliminates duplicate log messages when handling invalid blocks. While the performance and logging benefits only trigger during rare invalid block events or manual RPC invalidations, it clarifies validation control flow and untangles historical duplication noted in #31405 and #31533.

Membership: Directly alters ActivateBestChain, InvalidateBlock, and block index traversal routines in src/validation.cpp.

Factors: security/stability 0, bug 1, performance 1, user value 1, leverage 1

Reviewability: Ready

  • Ready for review
  • CI is passing and prior review concerns have been addressed

The PR has clean CI, is rebased, and all previous review comments have been addressed with a crACK on the latest push.

Author status: active; addressed review concerns and rebased on 2026-08-19

Resolved concerns:

  • Crypt-iQ noted that the initial push missed SetBlockFailureFlags and RecalculateBestHeader for InvalidateBlock when the block was not in the active chain; author reinstated these calls and added a regression test.
  • frankomosh noted changes to InvalidateBlock log formatting; author explained why dedicated RPC logging is preferable and not part of a stable API.

Agreement: Strong

  • Strong support for cleaning up redundant loops and logs in chain activation (Crypt-iQ, stratospher)
  • Concept approval of the refactoring approach (frankomosh, stratospher)
  • Prior concern about missing updates for inactive blocks resolved with a regression test (Crypt-iQ)

Strong; consensus that cleaning up duplicate loops and logs in ActivateBestChain is worthwhile (Crypt-iQ, stratospher).

Multiple contributors and maintainers support the refactoring. Crypt-iQ crACKed the latest push after prior concerns about out-of-chain blocks were resolved.

  • Crypt-iQ gave crACK ff06c05043bf: 'This PR by itself is a good improvement to ActivateBestChain.'
  • stratospher gave Concept ACK: 'nice cleanup!' and shared a summary sheet on block invalidity handling.
  • frankomosh gave Concept ACK on the refactoring approach.
  • bensig ACKed an earlier commit.

Review verdicts (DrahtBot): 1 (+1)

Files

File list not available for this run.

Card

This PR refactors invalid block handling across ActivateBestChain and InvalidateBlock to eliminate repeated traversals of the block index and duplicated log messages. Moving flag updates and header recalculations into InvalidBlockFound leaves UpdateBestInvalid to handle only its remaining role. It solves an inefficiency and logging nuisance identified in earlier validation PRs. Review is currently in good standing with a code review ACK from Crypt-iQ on the latest push and no open blockers.

Data

dossier JSON · extract JSON · model openrouter/google/gemini-3.8-flash, generated 2026-09-17T15:50, confidence high, input hash cc2c7422cd3687d2