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/32587.

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #28676 (Cluster mempool implementation by sdaftuar)

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.

feel free to ping me when it's ready

feel free to ping me when it's ready

Sure!

🚧 At least one of the CI tasks failed.
_{Task lint: https://github.com/bitcoin/bitcoin/runs/43164565056
LLM reason (✨ experimental): The CI failure is primarily due to errors from the lint check, specifically from ruff, which identified and flagged Python code issues such as unused imports and trailing whitespace.}

@instagibbs can you review this, will add other tests once this one gets ACK'ed

Addressed some nits and rebased with master!

update: rebased to master!

reACK 3afc731

just a reminder that you don't need to rebase on master unless a silent merge conflict is detected or drahtbot yells at you about an issue merging

git range-diff master 4ddc577953a7407ea44a3e3d8d9a4635929307ca 3afc73134bf652d01e25232701c2ef192b3fea0b

code review and tested-ACK

This PR is well designed: the refactoring to centralize fork creation and the switch to realistic reorg behavior both improve test clarity and fidelity.

• da836ca: Cleanly moves create_empty_fork into blocktools.py, enabling reuse across functional tests.
• 3afc731: Updates reorg logic in multiple tests to use actual fork submission rather than invalidateblock(), which better matches real‐world chain behavior.

One remaining question on duplication:

Across several test files, trigger_reorg is defined twice:

# Variation A: explicit node parameter
def trigger_reorg(fork_blocks, node):
    for block in fork_blocks:
        node.submitblock(block.serialize().hex())
    assert_equal(node.getbestblockhash(), fork_blocks[-1].hash)

# Variation B: hardcoded to self.nodes[0]
def trigger_reorg(self, fork_blocks):
    for block in fork_blocks:
        self.nodes[0].submitblock(block.serialize().hex())
    assert_equal(self.nodes[0].getbestblockhash(), fork_blocks[-1].hash)

Only the node lookup differs. Would it make sense to extract a single trigger_reorg(fork_blocks, node) utility in test_framework/blocktools.py and have all tests call that?

Across several test files, trigger_reorg is defined twice:

I am not sure if you have reviewed the code nicely, there isn't any duplication but rather a different approach using time-based timelock, see comment

Only the node lookup differs. Would it make sense to extract a single trigger_reorg(fork_blocks, node) utility in test_framework/blocktools.py and have all tests call that?

For now, I think we can keep like this. There are lot's of other tests as well which needs the reorg correction and can be done in a follow-up pr.

Thanks for the clarification! You’re absolutely right - I should have reviewed the implementation details more carefully. I appreciate you pointing out that these are different approaches (including the time-based timelock approach) rather than simple duplication.

Also, you’re correct about my phrasing - “defined twice” was unclear when it’s actually “defined multiple times across files.”

Makes sense to address this in a follow-up PR along with other tests that need reorg corrections.

update: in mempool_reorg.py: “slighty” -> “slightly” [correct spelling]

reACK a161f05

spelling fix

Rebased -> master@672c85cb1ea0d76ef9e596b2a964b2311b11b815

think you need to fix issues first

🚧 At least one of the CI tasks failed.
_{Task lint: https://github.com/bitcoin/bitcoin/runs/46275827051
LLM reason (✨ experimental): The CI failure is caused by errors detected by clang-tidy in the Python linter, specifically undefined names create_block and create_coinbase.}

Fixed conflicts and rebased to master.