test: Add functional tests for blockreconstructionextratxn and extra pool (compactblocks)

bigshiny90 commented at 4:40 pm on July 20, 2025: none

This tests the -blockreconstructionextratxn parameter and extra pool memory (ring buffer) behavior used specifically for compact block reconstruction. The “extra transaction pool” stores transactions that were rejected from the mempool for policy reasons (dust, low fees, non-standard scripts), txs replaced via rbf, etc…

There is no test coverage for this. (no unit tests either as the code being tested - in PeerManagerImpl - isn’t publicly accessible in the net_processing.h file)

The code being tested is in src/net_processing.cpp - look for vExtraTxnForCompact, AddToCompactExtraTransactions and -blockreconstructionextratxn start arg.

This tests

Policy-rejected transactions are stored in the extra pool: Transactions rejected for being dust, having low fees, or using non-standard scripts are kept in the extra pool Pool capacity limits work correctly: Tests various pool sizes (0, 1, 50, 400, 100000). Default is 100. Eviction behavior: When the extra pool is full, the oldest transactions are evicted (FIFO) Wraparound behavior: The extra pool correctly wraps around when adding new transactions Compact block reconstruction: These extra transactions are actually used during block reconstruction

Uses policy rejected transactions to populate the extra pool for tests.

DrahtBot added the label Tests on Jul 20, 2025

DrahtBot commented at 4:40 pm on July 20, 2025: 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/33023.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

DrahtBot added the label CI failed on Jul 20, 2025

DrahtBot commented at 6:35 pm on July 20, 2025: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/46342861746 LLM reason (✨ experimental): The CI failure is primarily caused by lint errors detected by ruff, including issues with trailing whitespace and possible spelling errors.

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

bigshiny90 force-pushed on Jul 23, 2025

fanquake marked this as a draft on Jul 24, 2025

bigshiny90 force-pushed on Jul 24, 2025

bigshiny90 commented at 11:31 am on July 24, 2025: none

updated to fix CI Lint errors

(also had to remove an unused param)

fanquake marked this as ready for review on Jul 24, 2025

bigshiny90 force-pushed on Jul 24, 2025

DrahtBot removed the label CI failed on Jul 24, 2025

brunoerg commented at 7:48 pm on July 24, 2025: contributor

I think the description is not clear about what you’re trying to cover/achieve with the test. Can you explain it better? Also, corecheck doesn’t show any related new coverage.

bigshiny90 commented at 8:11 pm on July 24, 2025: none

I think the description is not clear about what you’re trying to cover/achieve with the test. Can you explain it better? Also, corecheck doesn’t show any related new coverage.

Added better description (hopefully) to PR

I’m not sure about corecheck, but i’ll hazard a guess:

There are other tests that cover the p2p_compactblocks - so these tests seem to cover the same ground, though in reality they are testing something new. (also, the fact that most of what is being tested is not publicly exposed). Just a guess, as I don’t know how corecheck determines things

Hopefully answers the questions?

bigshiny90 renamed this:
~~test: Add functional tests for blockreconstructionextratxn parameter~~
test: Add functional tests for blockreconstructionextratxn and extra pool (compactblocks)
on Jul 25, 2025

in test/functional/p2p_compactblocks_extratxs.py:411 in 4985235dd2 outdated

406+        self.make_utxos()
407+
408+        # Ensure segwit is active
409+        assert softfork_active(self.nodes[0], "segwit")
410+
411+       # Test policy rejection types first

luke-jr commented at 9:18 pm on July 29, 2025:

Missing a space

bigshiny90 commented at 10:05 pm on July 29, 2025:

fixed. Thanks!

luke-jr changes_requested

bigshiny90 force-pushed on Jul 29, 2025

bigshiny90 requested review from brunoerg on Jul 29, 2025

bigshiny90 requested review from luke-jr on Jul 29, 2025

test: Add functional tests for blockreconstructionextratxn parameter

This adds comprehensive functional tests for the extra transaction pool
used in compact block reconstruction, controlled by the
-blockreconstructionextratxn parameter.

bd1c6b4ea9

bigshiny90 force-pushed on Jul 30, 2025

test: Add functional tests for blockreconstructionextratxn and extra pool (compactblocks) #33023

Code Coverage & Benchmarks

Reviews