avoid using invalidateblock to directly test reorg behavior #32531

1. 
   instagibbs commented at 1:54 pm on May 16, 2025: member

   Motivation

   While working on #32516 I found that for testing mempool entry from reorged blocks, directly using invalidateblock to trigger the reorg has at least a couple behaviors that don’t match real reorgs:

   1. Only allows 10 deep reorg before it stops trying to re-enter things into the mempool
   2. doesn’t respect descendant chain limits(Sets of PreChecks isn’t run once per reorg but once per block, letting longer chains slip in)

   the code paths greatly diverge from normal re-orgs, which makes this problematic for actual testing.

   The setup being used:

   1. build blockchain of stuff you want re-entry for
   2. invalidatblock to the desired fork off point

   One example to look at would be https://github.com/bitcoin/bitcoin/blob/3f83c744ac28b700090e15b5dda2260724a56f49/test/functional/mempool_ephemeral_dust.py#L350 or any other tests in mempool_*.py that use invalidateblock to simulate reorgs and mempool re-entry.

   Possible solution

   We should make sure that in our reorg/mempool tests spread across the codebase that we don’t do it this way and instead:

   1. Prep fork chain of sufficient length
   2. invalidate the fork chain
   3. do normal test setup
   4. reconsiderblock the previously invalidated chain

   This appears to match the intended test coverage.

   Useful Skills

   Understanding of python test suite

   Guidance for new contributors

   Want to work on this issue?

   For guidance on contributing, please read CONTRIBUTING.md before opening your pull request.

2. instagibbs added the label good first issue on May 16, 2025
3. 
   mzumsande commented at 2:47 pm on May 16, 2025: contributor

   We should make sure that in our reorg/mempool tests spread across the codebase that we don’t do it this way and instead:

   1. Prep fork chain of sufficient length
   2. invalidate the fork chain
   3. do normal test setup
   4. reconsiderblock the previously invalidated chain

   This appears to match the intended test coverage.

   Why not simply submit the fork chain at the right point in time, provided it has more work - no invalidateblock / reconsiderblock involved at all. I always thought the reason for using those was just the simplicity of not having to prepare a longer chain to reorg to.

4. 
   instagibbs commented at 2:50 pm on May 16, 2025: member

   Why not simply submit the fork chain at the right point in time

   Sure, that works too, it just seemed easiest since you can use ~all the tooling to generate blockchains. e.g., if you want stuff in the fork blocks (and make as many blocks as you like) it’s trivial to do via rpcs like generateblock.

5. instagibbs renamed this:
   Change functional tests to trigger reorg by reconsiderblock
   avoid using invalidateblock to directly test reorg behavior
   on May 16, 2025
6. 
   mzumsande commented at 3:13 pm on May 16, 2025: contributor

   Why not simply submit the fork chain at the right point in time

   Sure, that works too, it just seemed easiest since you can use ~all the tooling to generate blockchains. e.g., if you want stuff in the fork blocks (and make as many blocks as you like) it’s trivial to do via rpcs like generateblock.

   Oh ok I see now, as in https://github.com/bitcoin/bitcoin/pull/32516/commits/16af944a547bfea8f1f910938491a8ad11a47de5. Could also prepare the fork chain on a second node using whatever tooling, and connect the two at the right moment, but if you want to use just one node that works too.

7. 
   i-am-yuvi commented at 1:30 pm on May 22, 2025: contributor

   Motivation

   While working on #32516 I found that for testing mempool entry from reorged blocks, directly using invalidateblock to trigger the reorg has at least a couple behaviors that don’t match real reorgs:

   1. Only allows 10 deep reorg before it stops trying to re-enter things into the mempool
   2. doesn’t respect descendant chain limits(Sets of PreChecks isn’t run once per reorg but once per block, letting longer chains slip in)

   the code paths greatly diverge from normal re-orgs, which makes this problematic for actual testing.

   The setup being used:

   1. build blockchain of stuff you want re-entry for
   2. invalidatblock to the desired fork off point

   Possible solution

   We should make sure that in our reorg/mempool tests spread across the codebase that we don’t do it this way and instead:

   1. Prep fork chain of sufficient length
   2. invalidate the fork chain
   3. do normal test setup
   4. reconsiderblock the previously invalidated chain

   This appears to match the intended test coverage.

   Thanks @instagibbs for identifying the issue, I have opened a draft PR to address this. Will notify once its ready for review!

8. 
   instagibbs commented at 1:32 pm on May 22, 2025: member

   Also wondering aloud how we can accurately test deep reorgs on real networks, like a mainnet node for performance testing. Anyone have suggestions on this?
9. 
   spikeyrock commented at 8:20 am on May 24, 2025: none

   Maybe spin up two regtest nodes, mine a fork on one, and then connect them at just the right time to trigger the reorg. That way, the reorg happens “naturally” through P2P sync rather than forcing it.

Contributors
instagibbs mzumsande i-am-yuvi spikeyrock

Labels
good first issue