test: add tests for cluster chunks

HowHsu commented at 1:04 pm on December 12, 2025: none

Add tests for cluster chunks, including: - GetWorstMainChunk() tests, which picks the worst chunk from txgraph. - GetCurrentChunk(), which returns chunks in linearization order after calling it iteratively.

DrahtBot added the label Tests on Dec 12, 2025

DrahtBot commented at 1:04 pm on December 12, 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/34057.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	sipa, rkrux

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

everytime -> every time [“everytime” is nonstandard; the correct two-word form is “every time”.]

2025-12-30

fanquake commented at 1:07 pm on December 12, 2025: member

cc @sipa

brunoerg commented at 1:13 pm on December 12, 2025: contributor

FWIW txgraph has a lot of unkilled mutants (some in GetWorstMainChunk) that could be addressed: https://corecheck.dev/mutation/src/txgraph.cpp

HowHsu commented at 1:41 pm on December 12, 2025: none

FWIW txgraph has a lot of unkilled mutants (some in GetWorstMainChunk) that could be addressed: https://corecheck.dev/mutation/src/txgraph.cpp

Thanks, I’ll take a look.

in src/test/txgraph_tests.cpp:394 in a368232282

389+    graph->AddDependency(/*parent=*/refs[1], /*child=*/refs[2]);
390+    graph->AddDependency(/*parent=*/refs[2], /*child=*/refs[3]);
391+
392+    graph->SanityCheck();
393+
394+    BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::TOP), NUM_TOTAL_TX);

sipa commented at 2:26 pm on December 12, 2025:

I think you can test more by repeating these getters/checks in between transaction additions.

E.g.:

Add tx A.
Check GetTransactionCount, GetWorstMainChunk, GetIndividualFeerate.
Add tx B, and the B->A dependency.
Check GetTransactionCount, GetWorstMainChunk, GetIndividualFeerate again.
Add tx C, and the C->B dependency.
Check GetTransactionCount, GetWorstMainChunk, GetIndividualFeerate again.
Add tx D, and the D->C dependency.
Check GetTransactionCount, GetWorstMainChunk, GetIndividualFeerate again.

That way, I think you can merge txgraph_get_worst_chunk_basic_chain and txgraph_get_worst_chunk_chain2 into one test.

HowHsu commented at 2:47 pm on December 12, 2025:

True, thanks, Sipa.

sipa commented at 2:27 pm on December 12, 2025: member

Concept ACK.

Nice start, we can certainly use more unit tests in this area, as significant parts of txgraph are only tested through fuzz tests, If you want to add more, looking for unkilled mutants found by @brunoerg is a good idea.

HowHsu force-pushed on Dec 14, 2025

DrahtBot added the label CI failed on Dec 14, 2025

DrahtBot commented at 2:47 pm on December 14, 2025: contributor

🚧 At least one of the CI tasks failed. Task Windows-cross to x86_64, ucrt: https://github.com/bitcoin/bitcoin/actions/runs/20208904989/job/58011715272 LLM reason (✨ experimental): C++ compile error: conflicting declarations of ‘chunk’ in txgraph_tests.cpp causing test_bitcoin build failure.

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.

HowHsu force-pushed on Dec 14, 2025

HowHsu renamed this:
~~test: add some txgraph tests~~
test: add a test for GetWorstMainChunk()
on Dec 14, 2025

DrahtBot removed the label CI failed on Dec 14, 2025

in src/test/txgraph_tests.cpp:303 in 071b11125f

298+    for (size_t i = 0; i  < chunk_feerates.size(); i++)
299+        BOOST_CHECK(graph->GetIndividualFeerate(*chunk.first[i]) == chunk_feerates[i]);
300+
301+    BOOST_CHECK_EQUAL(chunk.second.fee, chunk_feerate.fee);
302+    BOOST_CHECK_EQUAL(chunk.second.size, chunk_feerate.size);
303+}

rkrux commented at 8:48 am on December 15, 2025:

In 071b11125fa3aa8bb4bdae04188f850850eded7d “test: add a chunk test for txgraph”

Can use some verbose naming to highlight the arguments are the expected ones.
Nit: Can consider converting this helper to a lambda inside the test case if it’s unlikely to be used in other tests.

 0diff --git a/src/test/txgraph_tests.cpp b/src/test/txgraph_tests.cpp
 1index f4c367f4e2..4e97021ca1 100644
 2--- a/src/test/txgraph_tests.cpp
 3+++ b/src/test/txgraph_tests.cpp
 4@@ -291,17 +291,6 @@ BOOST_AUTO_TEST_CASE(txgraph_trim_big_singletons)
 5     }
 6 }
 7 
 8-void chunk_check_helper(std::unique_ptr<TxGraph>& graph, const std::vector<FeePerWeight>& chunk_feerates, FeePerWeight chunk_feerate)
 9-{
10-    auto chunk = graph->GetWorstMainChunk();
11-    BOOST_CHECK_EQUAL(chunk.first.size(), chunk_feerates.size());
12-    for (size_t i = 0; i  < chunk_feerates.size(); i++)
13-        BOOST_CHECK(graph->GetIndividualFeerate(*chunk.first[i]) == chunk_feerates[i]);
14-
15-    BOOST_CHECK_EQUAL(chunk.second.fee, chunk_feerate.fee);
16-    BOOST_CHECK_EQUAL(chunk.second.size, chunk_feerate.size);
17-}
18-
19 BOOST_AUTO_TEST_CASE(txgraph_get_worst_chunk_chain)
20 {
21     /** The maximum cluster count used in this test. */
22@@ -314,6 +303,16 @@ BOOST_AUTO_TEST_CASE(txgraph_get_worst_chunk_chain)
23     // Create a new graph for the test.
24     auto graph = MakeTxGraph(MAX_CLUSTER_COUNT, MAX_CLUSTER_SIZE, NUM_ACCEPTABLE_ITERS);
25 
26+    auto chunk_check_helper = [&graph](const std::vector<FeePerWeight>& expected_chunk_txs_feerates, FeePerWeight expected_chunk_feerate) {
27+        auto chunk = graph->GetWorstMainChunk();
28+        BOOST_CHECK_EQUAL(chunk.first.size(), expected_chunk_txs_feerates.size());
29+        for (size_t i = 0; i  < expected_chunk_txs_feerates.size(); i++)
30+            BOOST_CHECK(graph->GetIndividualFeerate(*chunk.first[i]) == expected_chunk_txs_feerates[i]);
31+
32+        BOOST_CHECK_EQUAL(chunk.second.fee, expected_chunk_feerate.fee);
33+        BOOST_CHECK_EQUAL(chunk.second.size, expected_chunk_feerate.size);
34+    };
35+
36     std::vector<TxGraph::Ref> refs;
37     refs.reserve(NUM_TOTAL_TX);
38 
39@@ -326,24 +325,24 @@ BOOST_AUTO_TEST_CASE(txgraph_get_worst_chunk_chain)
40     // [A]
41     refs.push_back(graph->AddTransaction(feerateA));
42     BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::TOP), 1);
43-    chunk_check_helper(graph, {feerateA}, feerateA);
44+    chunk_check_helper({feerateA}, feerateA);
45     // [A, B]
46     refs.push_back(graph->AddTransaction(feerateB));
47     graph->AddDependency(/*parent=*/refs[0], /*child=*/refs[1]);
48     BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::TOP), 2);
49-    chunk_check_helper(graph, {feerateB}, feerateB);
50+    chunk_check_helper({feerateB}, feerateB);
51 
52     // [A, BC]
53     refs.push_back(graph->AddTransaction(feerateC));
54     graph->AddDependency(/*parent=*/refs[1], /*child=*/refs[2]);
55     BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::TOP), 3);
56-    chunk_check_helper(graph, {feerateC, feerateB}, FeePerWeight{3, 20});
57+    chunk_check_helper({feerateC, feerateB}, FeePerWeight{3, 20});
58 
59     // [ABCD]
60     refs.push_back(graph->AddTransaction(feerateD));
61     graph->AddDependency(/*parent=*/refs[2], /*child=*/refs[3]);
62     BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::TOP), 4);
63-    chunk_check_helper(graph, {feerateD, feerateC, feerateB, feerateA}, FeePerWeight{9, 40});
64+    chunk_check_helper({feerateD, feerateC, feerateB, feerateA}, FeePerWeight{9, 40});
65 
66     graph->SanityCheck();
67 }

HowHsu commented at 8:23 am on December 16, 2025:

Thanks, applied this patch.

rkrux commented at 8:48 am on December 15, 2025: contributor

Concept ACK 071b111

test: add a chunk test for txgraph

Add a test for GetWorstMainChunk(), which picks the worst chunk from
txgraph.

Co-developed-by: rkrux <rkrux.connect@gmail.com>

a174737d03

HowHsu force-pushed on Dec 16, 2025

HowHsu requested review from sipa on Dec 16, 2025

HowHsu requested review from rkrux on Dec 16, 2025

test: add block builder tests for txgraph

Add block builder tests to make sure chunks for a cluster are all right.

d01cb98bf3

HowHsu renamed this:
~~test: add a test for GetWorstMainChunk()~~
test: add tests for cluster chunks
on Dec 25, 2025

test: add a test for txgraph staging

staging is a batching mechanism for txgraph, add a test for this
feature.

27f72bc43e

test: add tests for cluster chunks #34057

Code Coverage & Benchmarks

Reviews

LLM Linter (✨ experimental)