test: add tests for cluster chunks #34057

pull HowHsu wants to merge 3 commits into bitcoin:master from HowHsu:get_worst_chunk changing 1 files +130 −0
  1. HowHsu commented at 1:04 PM on December 12, 2025: none

    Add tests for cluster chunks, including: - txgraph_chunk_chain test: test chunk implementation for a simple chain style graph . - txgraph_staging test: test the staging feature for a basic graph.

  2. DrahtBot added the label Tests on Dec 12, 2025
  3. DrahtBot commented at 1:04 PM on December 12, 2025: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34057.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK instagibbs, sipa
    Concept ACK rkrux

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

    LLM Linter (✨ experimental)

    Possible typos and grammar issues:

    • everytime -> every time [“everytime” is nonstandard; two words “every time” is correct]
    • everytime -> every time [same correction in the second occurrence]

    <sup>2026-02-07 13:08:00</sup>

  4. fanquake commented at 1:07 PM on December 12, 2025: member

    cc @sipa

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

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

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

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

  9. HowHsu force-pushed on Dec 14, 2025
  10. DrahtBot added the label CI failed on Dec 14, 2025
  11. DrahtBot commented at 2:47 PM on December 14, 2025: contributor

    <!--85328a0da195eb286784d51f73fa0af9-->

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

    <details><summary>Hints</summary>

    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.

    </details>

  12. HowHsu force-pushed on Dec 14, 2025
  13. HowHsu renamed this:
    test: add some txgraph tests
    test: add a test for GetWorstMainChunk()
    on Dec 14, 2025
  14. DrahtBot removed the label CI failed on Dec 14, 2025
  15. 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"

    1. Can use some verbose naming to highlight the arguments are the expected ones.
    2. Nit: Can consider converting this helper to a lambda inside the test case if it's unlikely to be used in other tests.
    diff --git a/src/test/txgraph_tests.cpp b/src/test/txgraph_tests.cpp
index f4c367f4e2..4e97021ca1 100644
--- a/src/test/txgraph_tests.cpp
+++ b/src/test/txgraph_tests.cpp
@@ -291,17 +291,6 @@ BOOST_AUTO_TEST_CASE(txgraph_trim_big_singletons)
     }
 }
 
-void chunk_check_helper(std::unique_ptr<TxGraph>& graph, const std::vector<FeePerWeight>& chunk_feerates, FeePerWeight chunk_feerate)
-{
-    auto chunk = graph->GetWorstMainChunk();
-    BOOST_CHECK_EQUAL(chunk.first.size(), chunk_feerates.size());
-    for (size_t i = 0; i  < chunk_feerates.size(); i++)
-        BOOST_CHECK(graph->GetIndividualFeerate(*chunk.first[i]) == chunk_feerates[i]);
-
-    BOOST_CHECK_EQUAL(chunk.second.fee, chunk_feerate.fee);
-    BOOST_CHECK_EQUAL(chunk.second.size, chunk_feerate.size);
-}
-
 BOOST_AUTO_TEST_CASE(txgraph_get_worst_chunk_chain)
 {
     /** The maximum cluster count used in this test. */
@@ -314,6 +303,16 @@ BOOST_AUTO_TEST_CASE(txgraph_get_worst_chunk_chain)
     // Create a new graph for the test.
     auto graph = MakeTxGraph(MAX_CLUSTER_COUNT, MAX_CLUSTER_SIZE, NUM_ACCEPTABLE_ITERS);
 
+    auto chunk_check_helper = [&graph](const std::vector<FeePerWeight>& expected_chunk_txs_feerates, FeePerWeight expected_chunk_feerate) {
+        auto chunk = graph->GetWorstMainChunk();
+        BOOST_CHECK_EQUAL(chunk.first.size(), expected_chunk_txs_feerates.size());
+        for (size_t i = 0; i  < expected_chunk_txs_feerates.size(); i++)
+            BOOST_CHECK(graph->GetIndividualFeerate(*chunk.first[i]) == expected_chunk_txs_feerates[i]);
+
+        BOOST_CHECK_EQUAL(chunk.second.fee, expected_chunk_feerate.fee);
+        BOOST_CHECK_EQUAL(chunk.second.size, expected_chunk_feerate.size);
+    };
+
     std::vector<TxGraph::Ref> refs;
     refs.reserve(NUM_TOTAL_TX);
 
@@ -326,24 +325,24 @@ BOOST_AUTO_TEST_CASE(txgraph_get_worst_chunk_chain)
     // [A]
     refs.push_back(graph->AddTransaction(feerateA));
     BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::TOP), 1);
-    chunk_check_helper(graph, {feerateA}, feerateA);
+    chunk_check_helper({feerateA}, feerateA);
     // [A, B]
     refs.push_back(graph->AddTransaction(feerateB));
     graph->AddDependency(/*parent=*/refs[0], /*child=*/refs[1]);
     BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::TOP), 2);
-    chunk_check_helper(graph, {feerateB}, feerateB);
+    chunk_check_helper({feerateB}, feerateB);
 
     // [A, BC]
     refs.push_back(graph->AddTransaction(feerateC));
     graph->AddDependency(/*parent=*/refs[1], /*child=*/refs[2]);
     BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::TOP), 3);
-    chunk_check_helper(graph, {feerateC, feerateB}, FeePerWeight{3, 20});
+    chunk_check_helper({feerateC, feerateB}, FeePerWeight{3, 20});
 
     // [ABCD]
     refs.push_back(graph->AddTransaction(feerateD));
     graph->AddDependency(/*parent=*/refs[2], /*child=*/refs[3]);
     BOOST_CHECK_EQUAL(graph->GetTransactionCount(TxGraph::Level::TOP), 4);
-    chunk_check_helper(graph, {feerateD, feerateC, feerateB, feerateA}, FeePerWeight{9, 40});
+    chunk_check_helper({feerateD, feerateC, feerateB, feerateA}, FeePerWeight{9, 40});
 
     graph->SanityCheck();
 }
    HowHsu commented at 8:23 AM on December 16, 2025:

    Thanks, applied this patch.

  16. rkrux commented at 8:48 AM on December 15, 2025: contributor

    Concept ACK 071b111

  17. HowHsu force-pushed on Dec 16, 2025
  18. HowHsu requested review from sipa on Dec 16, 2025
  19. HowHsu requested review from rkrux on Dec 16, 2025
  20. HowHsu renamed this:
    test: add a test for GetWorstMainChunk()
    test: add tests for cluster chunks
    on Dec 25, 2025
  21. in src/test/txgraph_tests.cpp:306 in d01cb98bf3 
     302 | @@ -303,14 +303,41 @@ BOOST_AUTO_TEST_CASE(txgraph_get_worst_chunk_chain)
 303 |      // Create a new graph for the test.
 304 |      auto graph = MakeTxGraph(MAX_CLUSTER_COUNT, MAX_CLUSTER_SIZE, NUM_ACCEPTABLE_ITERS);
 305 |  
 306 | -    auto chunk_check_helper = [&graph](const std::vector<FeePerWeight>& expected_chunk_txs_feerates, FeePerWeight expected_chunk_feerate) {
 307 | +    auto worst_chunk_checker = [&graph](const std::vector<FeePerWeight>& expected_chunk_txs_feerates, FeePerWeight expected_chunk_feerate) {
    sipa commented at 6:12 PM on February 3, 2026:

    In commit "test: add block builder tests for txgraph"

    I think this worst_chunk_checker is made redundant by the addition of block_builder_checker, as that function checks that the first GetWorstChunk() matches the last block builder chunk anyway.

    HowHsu commented at 6:49 AM on February 4, 2026:

    In commit "test: add block builder tests for txgraph"

    I think this worst_chunk_checker is made redundant by the addition of block_builder_checker, as that function checks that the first GetWorstChunk() matches the last block builder chunk anyway.

    Hi Sipa, I got a fever thiese days, I'll be back looking into this days later, sorry for the delay.

    HowHsu commented at 8:33 AM on February 5, 2026:

    In commit "test: add block builder tests for txgraph"

    I think this worst_chunk_checker is made redundant by the addition of block_builder_checker, as that function checks that the first GetWorstChunk() matches the last block builder chunk anyway.

    right, I've deleted that closure.

  22. HowHsu force-pushed on Feb 5, 2026
  23. HowHsu requested review from sipa on Feb 6, 2026
  24. sipa commented at 4:45 PM on February 6, 2026: member

    ACK 4e549f7a355024a8035218274a545d23a09e7966

  25. fanquake commented at 5:13 PM on February 6, 2026: member

    cc @instagibbs

  26. in src/test/txgraph_tests.cpp:385 in 4e549f7a35 
     380 | +    /** The maximum cluster count used in this test. */
 381 | +    static constexpr int MAX_CLUSTER_COUNT = 50;
 382 | +    /** The total number of transactions in the test. */
 383 | +    static constexpr int NUM_TOTAL_TX = 2;
 384 | +    /** Set a very large cluster size limit so that only the count limit is triggered. */
 385 | +    static constexpr int32_t MAX_CLUSTER_SIZE = 100'000 * 100;
    instagibbs commented at 5:28 PM on February 6, 2026:

    4e549f7a355024a8035218274a545d23a09e7966

    nit: Seems a little large unless we can't control input sizes for some reason.

    HowHsu commented at 1:02 PM on February 7, 2026:

    4e549f7

    nit: Seems a little large unless we can't control input sizes for some reason.

    Right, I've make it tighter

  27. in src/test/txgraph_tests.cpp:377 in 4e549f7a35 outdated 
     374 | @@ -375,4 +375,62 @@ BOOST_AUTO_TEST_CASE(txgraph_chunk_chain)
 375 |      block_builder_checker({{&refs[0]}});
 376 |  }
 377 |  
 378 | +BOOST_AUTO_TEST_CASE(txgraph_staging)
 379 | +{
 380 | +    /** The maximum cluster count used in this test. */
 381 | +    static constexpr int MAX_CLUSTER_COUNT = 50;
 382 | +    /** The total number of transactions in the test. */
 383 | +    static constexpr int NUM_TOTAL_TX = 2;
    instagibbs commented at 5:29 PM on February 6, 2026:

    4e549f7a355024a8035218274a545d23a09e7966

    nit: using this only to reserve something once is probably overkill

    HowHsu commented at 1:08 PM on February 7, 2026:

    4e549f7

    nit: using this only to reserve something once is probably overkill

    Done.

  28. instagibbs commented at 5:32 PM on February 6, 2026: member

    ACK 4e549f7a355024a8035218274a545d23a09e7966

    non-blocking nits only

    Some simple examples can be helpful for new reviewers as a form of documentation and likely a good start to aiming at more specific things the fuzz-led targets may not reliably hit. (I need to learn how to read the mutants report...)

  29. HowHsu force-pushed on Feb 7, 2026
  30. HowHsu force-pushed on Feb 7, 2026
  31. DrahtBot added the label CI failed on Feb 7, 2026
  32. 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>
    4a1ac31e97
  33. test: add block builder tests for txgraph
    Add block builder tests to make sure chunks for a cluster are all right.
    ef253a9d3d
  34. test: add a test for txgraph staging
    staging is a batching mechanism for txgraph, add a test for this
feature.
    fe0b1513a7
  35. HowHsu force-pushed on Feb 7, 2026
  36. fanquake added this to the milestone 31.0 on Feb 7, 2026
  37. DrahtBot removed the label CI failed on Feb 7, 2026
  38. HowHsu requested review from instagibbs on Feb 9, 2026
  39. HowHsu requested review from sipa on Feb 9, 2026
  40. in src/test/txgraph_tests.cpp:376 in fe0b1513a7 
     371 | +BOOST_AUTO_TEST_CASE(txgraph_staging)
 372 | +{
 373 | +    /* Create a new graph for the test.
 374 | +     * The parameters are max_cluster_count, max_cluster_size, acceptable_iters
 375 | +     */
 376 | +    auto graph = MakeTxGraph(10, 1000, NUM_ACCEPTABLE_ITERS);
    instagibbs commented at 2:50 PM on February 9, 2026:

    alternatively to naming them in above comment

        auto graph = MakeTxGraph(/*max_cluster_count=*/10, /*max_cluster_size=*/1000, NUM_ACCEPTABLE_ITERS);
  41. instagibbs approved
  42. instagibbs commented at 2:53 PM on February 9, 2026: member

    reACK fe0b1513a7c53b8490b81165acf1c7d42297a2ed

    No need to touch commits unless other issues are surfaced.

  43. sipa commented at 3:19 PM on February 9, 2026: member

    reACK fe0b1513a7c53b8490b81165acf1c7d42297a2ed

  44. fanquake merged this on Feb 9, 2026
  45. fanquake closed this on Feb 9, 2026
Contributors
HowHsuDrahtBotfanquakebrunoergsiparkruxinstagibbs


rkrux

Labels
Tests

Milestone
31.0

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-22 09:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me