test: Add functional tests for blockreconstructionextratxn and extra pool (compactblocks) by bigshiny90 · Pull Request #33023 · bitcoin/bitcoin

bigshiny90 · 2025-07-20T16:40:28Z

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

(Updated 08/29/25: rebased to currrent master and fixed tests to accommodate new low-fee rate policy)

DrahtBot · 2025-07-20T16:40:33Z

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.

Type	Reviewers
ACK	bensig

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

so we can eg wait until a particular block is announced. -> # so we can e.g. wait until a particular block is announced. [Uses standard abbreviation punctuation for "for example", improving clarity]

^{drahtbot_id_5_m}

DrahtBot · 2025-07-20T18:35:08Z

🚧 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.}

Hints

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 · 2025-07-24T11:31:19Z

updated to fix CI Lint errors

(also had to remove an unused param)

brunoerg

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 · 2025-07-24T20:11:44Z

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?

This adds comprehensive functional tests for the extra transaction pool used in compact block reconstruction, controlled by the -blockreconstructionextratxn parameter. Github-Pull: bitcoin#33023 Rebased-From: bd1c6b4ea9b94ab4b2604f2a4b8a7fd1fe55c92a

kevkevinpal

I get the idea of testing the extra pool. I think some of the tests can be consolidated.

I did run the test locally, and it passes for me. I haven't checked code coverage either to see if it covers anything extra,

but grep -nri "blockreconstructionextratxn" ./test/functional/ does not have any matches

kevkevinpal · 2025-08-25T21:55:36Z

This seems misplaced for this test test_extratxn_invalid_parameters

removed. now just tests for invalid capacity -1

kevkevinpal · 2025-08-25T21:58:10Z

instead of 8 can you make this num_txs - count, it makes sense to not have magic numbers

kevkevinpal · 2025-08-25T22:03:53Z

what is the point of testing with 2 when you have another test, testing with 1?

yes, removed redundant test - test_extratxn_minimal_capacity_eviction

kevkevinpal · 2025-08-25T22:07:09Z

If you could why not set buffersize to 400, and then after checking the capacity test the wrap around?

Then you can drop test_extratxn_large_capacity and test_extratxn_buffer_wraparound seems redundant to do the setup multiple times when they can be done in the same test.

I combined the 3 separate tests - test_extratxnpool_capacity, test_extratxn_large_capacity, test_extratxn_buffer_wraparound - and created one test test_extratxnpool_capacity_and_wraparound, which tests capacity 400 and wraparound behavior

This adds comprehensive functional tests for the extra transaction pool used in compact block reconstruction, controlled by the -blockreconstructionextratxn parameter. Github-Pull: bitcoin#33023 Rebased-From: f9c6331cb26f3bafc3dba7f5a72622a9ff4f190d

DrahtBot · 2025-08-29T07:51:11Z

Could turn into draft while CI is red?

bigshiny90 · 2025-08-29T13:29:14Z

investigating why tests fail on github CI... but runs fine on a local build

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

bigshiny90 · 2025-08-29T14:02:40Z

needed to rebase to current master and fix tests for the new lower fee-rate policy.

bensig · 2026-01-02T23:49:40Z

ACK 841b3c2

Tested locally on macOS - all test cases pass.

Good coverage for the extra transaction pool used in compact block reconstruction. Tests policy-rejected txs (dust, low_fee, nonstandard), pool capacity limits, FIFO eviction, and edge cases.

nit: The ✓ emojis in log messages are a bit unconventional compared to other functional tests, but doesn't affect functionality

sedited · 2026-02-09T12:28:21Z

+        result = self.send_compact_block(tx_list, indices)
+
+        assert_equal(result["missing_indices"], [])
+        self.log.info("✓ All rejected transactions are available in extra pool")


All these info messages on completion should be removed in my opinion.

sedited · 2026-02-09T12:38:56Z

+        # Check original transactions again - first one should be evicted
+        result2 = self.send_compact_block(rejected_txs, indices)
+        assert_equal(result2["missing_indices"], [0])
+        self.log.info("✓ Transaction 0 was evicted as expected (wraparound)")


All these info messages on test completion should be removed imo.

sedited · 2026-04-19T07:47:35Z

Are you still working on this? There are unaddressed review comments.

fanquake · 2026-06-01T13:46:12Z

Closing for now. Let us know if you're coming back to this.

DrahtBot added the Tests label Jul 20, 2025

DrahtBot added the CI failed label Jul 20, 2025

bigshiny90 force-pushed the compactblocks-extratxs-tests-core branch from 76f1896 to 3e93378 Compare July 23, 2025 11:51

fanquake marked this pull request as draft July 24, 2025 10:06

bigshiny90 force-pushed the compactblocks-extratxs-tests-core branch 3 times, most recently from 90f0f1d to a01547c Compare July 24, 2025 11:30

fanquake marked this pull request as ready for review July 24, 2025 11:47

bigshiny90 force-pushed the compactblocks-extratxs-tests-core branch from a01547c to 4985235 Compare July 24, 2025 12:16

DrahtBot removed the CI failed label Jul 24, 2025

brunoerg reviewed Jul 24, 2025

View reviewed changes

bigshiny90 changed the title ~~test: Add functional tests for blockreconstructionextratxn parameter~~ test: Add functional tests for blockreconstructionextratxn and extra pool (compactblocks) Jul 25, 2025

luke-jr suggested changes Jul 29, 2025

View reviewed changes

Comment thread test/functional/p2p_compactblocks_extratxs.py Outdated

bigshiny90 force-pushed the compactblocks-extratxs-tests-core branch from 4985235 to b4f34aa Compare July 29, 2025 22:02

bigshiny90 requested review from brunoerg and luke-jr July 29, 2025 22:05

bigshiny90 force-pushed the compactblocks-extratxs-tests-core branch from b4f34aa to bd1c6b4 Compare July 30, 2025 20:13

kevkevinpal reviewed Aug 25, 2025

View reviewed changes

bigshiny90 force-pushed the compactblocks-extratxs-tests-core branch from bd1c6b4 to f9c6331 Compare August 25, 2025 23:40

bigshiny90 requested a review from kevkevinpal August 25, 2025 23:46

DrahtBot added the CI failed label Aug 26, 2025

test: Add functional tests for blockreconstructionextratxn parameter

841b3c2

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

bigshiny90 force-pushed the compactblocks-extratxs-tests-core branch from f9c6331 to 841b3c2 Compare August 29, 2025 13:59

DrahtBot removed the CI failed label Aug 29, 2025

sedited reviewed Feb 9, 2026

View reviewed changes

fanquake marked this pull request as draft May 7, 2026 10:08

fanquake closed this Jun 1, 2026

Uh oh!

Conversation

bigshiny90 commented Jul 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jul 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

LLM Linter (✨ experimental)

so we can eg wait until a particular block is announced. -> # so we can e.g. wait until a particular block is announced. [Uses standard abbreviation punctuation for "for example", improving clarity]

Uh oh!

DrahtBot commented Jul 20, 2025

Uh oh!

bigshiny90 commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brunoerg left a comment

Choose a reason for hiding this comment

Uh oh!

bigshiny90 commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

kevkevinpal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bigshiny90 Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Aug 29, 2025

Uh oh!

bigshiny90 commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bigshiny90 commented Aug 29, 2025

Uh oh!

bensig commented Jan 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sedited commented Apr 19, 2026

Uh oh!

fanquake commented Jun 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

bigshiny90 commented Jul 20, 2025 •

edited

Loading

DrahtBot commented Jul 20, 2025 •

edited

Loading

bigshiny90 commented Jul 24, 2025 •

edited

Loading

bigshiny90 commented Jul 24, 2025 •

edited

Loading

bigshiny90 Aug 25, 2025 •

edited

Loading

bigshiny90 commented Aug 29, 2025 •

edited

Loading