Shallow invalid forks + ActivateBestChainStep result in overly aggressive mempool filtering #32838

1. 
   instagibbs commented at 5:05 pm on June 30, 2025: member

   Is there an existing issue for this?

   • I have searched the existing issues

   Current behaviour

   Upon reorgs, the mempool has its transactions filtered near the end of every call of ActivateBestChainStep via MaybeUpdateMempoolForReorg : https://github.com/bitcoin/bitcoin/blob/a763497b1d6661bc5a7b8943d8f04a4459fd6827/src/validation.cpp#L3403

   If the fork being activated is actually invalid this can happen:

   1. Chain at height T, with transaction in mempool which relies on height T or higher for lock validity (nsequence f.e.)
   2. Alternative chain to height T+1, forking at T-1 is encountered.
   3. ActivateBestChainStep fires once, rolling back to height T-1, then stopping when it attempts to validate the invalid block fork at height T.
   4. Since fBlocksDisconnected is true, MaybeUpdateMempoolForReorg is called at height T-1
   5. Transaction is filtered due to non-final locks at height T-1, and function returns
   6. Next ActivateBestChainStep is called, but with an empty disconnect pool, getting back to height T with original chain tip at step (1)

   I think this can also happen if a valid reorg is longer than 32 blocks: https://github.com/bitcoin/bitcoin/blob/a763497b1d6661bc5a7b8943d8f04a4459fd6827/src/validation.cpp#L3360

   Expected behaviour

   Ideally, the disconnectpool would not be re-applied until the known reorg is complete.

   Steps to reproduce

   An example causing this was found during debugging a related fuzzing crash: #28676 (comment)

   Relevant log output

   No response

   How did you obtain Bitcoin Core

   Compiled from source

   What version of Bitcoin Core are you using?

   https://github.com/dergoegge/bitcoin/commit/6329ce979f63b396aa036a1ad39798bb83fa4ade

   Operating system and version

   Ubuntu

   Machine specifications

   No response

2. 
   instagibbs commented at 5:06 pm on June 30, 2025: member

   cc @dergoegge
3. 
   mzumsande commented at 6:27 pm on June 30, 2025: contributor

   Mabe we could create a DisconnectedBlockTransactions pool in ActivateBestChain() that is passed to each ActivateBestChainStep() instead of having a separate one in each ActivateBestChainStep() call, and then only call MaybeUpdateMempoolForReorg after ABC has made enough progress. We’d have to deal with blocks from multiple separate disconnections within one pool then (meaning duplicates/ possbile conflicts, because we could go into multiple forks, try to connect blocks, fail at some point along the way, disconnect those blocks again and so on) This might complicate things a bit.

   Obviously someone has to create invalid blocks with valid PoW to trigger this (losing at least one block reward), which is costly.

Contributors
instagibbs mzumsande