Merge bitcoin/bitcoin#29757: feefrac: avoid explicitly computing diagram; compare based on chunks

b22901dfa9cc3af94bf13163a28300eb1ab25b22 Avoid explicitly computing diagram; compare based on chunks (Pieter Wuille)

Pull request description:

  This merges the `BuildDiagramFromChunks` and `CompareFeeRateDiagram` introduced in #29242 into a single `CompareChunks` function, which operates on sorted chunk data rather than diagrams, instead computing the diagram on the fly.

  This avoids the need for the construction of an intermediary diagram object, and removes the slightly arbitrary "all diagrams must start at (0, 0)" requirement.

  Not a big deal, but I think the result is a bit cleaner and not really more complicated.

ACKs for top commit:
  glozow:
    reACK b22901d
  instagibbs:
    reACK https://github.com/bitcoin/bitcoin/pull/29757/commits/b22901dfa9cc3af94bf13163a28300eb1ab25b22

Tree-SHA512: ca37bdf61d9a9cb5435f4da73e97ead33bf65828ad9af49b87336b1ece70db8ced1c21f517fc6eb6d616311c91f3da75ecae6b9bd42547133e3a3c5320b7816d

4 files changed, 112 insertions, 145 deletions

diff --git a/src/test/feefrac_tests.cpp b/src/test/feefrac_tests.cpp
index 2e015b382e..5af3c3d7ed 100644
--- a/src/test/feefrac_tests.cpp
+++ b/src/test/feefrac_tests.cpp
@@ -82,43 +82,4 @@ BOOST_AUTO_TEST_CASE(feefrac_operators)
 
 }
 
-BOOST_AUTO_TEST_CASE(build_diagram_test)
-{
-    FeeFrac p1{1000, 100};
-    FeeFrac empty{0, 0};
-    FeeFrac zero_fee{0, 1};
-    FeeFrac oversized_1{4611686000000, 4000000};
-    FeeFrac oversized_2{184467440000000, 100000};
-
-    // Diagram-building will reorder chunks
-    std::vector<FeeFrac> chunks;
-    chunks.push_back(p1);
-    chunks.push_back(zero_fee);
-    chunks.push_back(empty);
-    chunks.push_back(oversized_1);
-    chunks.push_back(oversized_2);
-
-    // Caller in charge of sorting chunks in case chunk size limit
-    // differs from cluster size limit
-    std::sort(chunks.begin(), chunks.end(), [](const FeeFrac& a, const FeeFrac& b) { return a > b; });
-
-    // Chunks are now sorted in reverse order (largest first)
-    BOOST_CHECK(chunks[0] == empty); // empty is considered "highest" fee
-    BOOST_CHECK(chunks[1] == oversized_2);
-    BOOST_CHECK(chunks[2] == oversized_1);
-    BOOST_CHECK(chunks[3] == p1);
-    BOOST_CHECK(chunks[4] == zero_fee);
-
-    std::vector<FeeFrac> generated_diagram{BuildDiagramFromChunks(chunks)};
-    BOOST_CHECK(generated_diagram.size() == 1 + chunks.size());
-
-    // Prepended with an empty, then the chunks summed in order as above
-    BOOST_CHECK(generated_diagram[0] == empty);
-    BOOST_CHECK(generated_diagram[1] == empty);
-    BOOST_CHECK(generated_diagram[2] == oversized_2);
-    BOOST_CHECK(generated_diagram[3] == oversized_2 + oversized_1);
-    BOOST_CHECK(generated_diagram[4] == oversized_2 + oversized_1 + p1);
-    BOOST_CHECK(generated_diagram[5] == oversized_2 + oversized_1 + p1 + zero_fee);
-}
-
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/fuzz/feeratediagram.cpp b/src/test/fuzz/feeratediagram.cpp
index 6d710093cb..1a9c5ee946 100644
--- a/src/test/fuzz/feeratediagram.cpp
+++ b/src/test/fuzz/feeratediagram.cpp
@@ -16,6 +16,20 @@
 
 namespace {
 
+/** Takes the pre-computed and topologically-valid chunks and generates a fee diagram which starts at FeeFrac of (0, 0) */
+std::vector<FeeFrac> BuildDiagramFromChunks(const Span<const FeeFrac> chunks)
+{
+    std::vector<FeeFrac> diagram;
+    diagram.reserve(chunks.size() + 1);
+
+    diagram.emplace_back(0, 0);
+    for (auto& chunk : chunks) {
+        diagram.emplace_back(diagram.back() + chunk);
+    }
+    return diagram;
+}
+
+
 /** Evaluate a diagram at a specific size, returning the fee as a fraction.
  *
  * Fees in diagram cannot exceed 2^32, as the returned evaluation could overflow
@@ -103,7 +117,7 @@ FUZZ_TARGET(build_and_compare_feerate_diagram)
     assert(diagram1.front() == empty);
     assert(diagram2.front() == empty);
 
-    auto real = CompareFeerateDiagram(diagram1, diagram2);
+    auto real = CompareChunks(chunks1, chunks2);
     auto sim = CompareDiagrams(diagram1, diagram2);
     assert(real == sim);
 
diff --git a/src/test/fuzz/rbf.cpp b/src/test/fuzz/rbf.cpp
index 30ed148c37..64785948f6 100644
--- a/src/test/fuzz/rbf.cpp
+++ b/src/test/fuzz/rbf.cpp
@@ -82,17 +82,6 @@ FUZZ_TARGET(rbf, .init = initialize_rbf)
     }
 }
 
-void CheckDiagramConcave(std::vector<FeeFrac>& diagram)
-{
-    // Diagrams are in monotonically-decreasing feerate order.
-    FeeFrac last_chunk = diagram.front();
-    for (size_t i = 1; i<diagram.size(); ++i) {
-        FeeFrac next_chunk = diagram[i] - diagram[i-1];
-        assert(next_chunk <= last_chunk);
-        last_chunk = next_chunk;
-    }
-}
-
 FUZZ_TARGET(package_rbf, .init = initialize_package_rbf)
 {
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
@@ -107,7 +96,7 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf)
     std::vector<CTransaction> mempool_txs;
     size_t iter{0};
 
-    int64_t replacement_vsize = fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(1, 1000000);
+    int32_t replacement_vsize = fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(1, 1000000);
 
     // Keep track of the total vsize of CTxMemPoolEntry's being added to the mempool to avoid overflow
     // Add replacement_vsize since this is added to new diagram during RBF check
@@ -167,32 +156,33 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf)
         pool.CalculateDescendants(txiter, all_conflicts);
     }
 
-    // Calculate the feerate diagrams for a replacement.
+    // Calculate the chunks for a replacement.
     CAmount replacement_fees = ConsumeMoney(fuzzed_data_provider);
-    auto calc_results{pool.CalculateFeerateDiagramsForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)};
+    auto calc_results{pool.CalculateChunksForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)};
 
     if (calc_results.has_value()) {
-        // Sanity checks on the diagrams.
+        // Sanity checks on the chunks.
 
-        // Diagrams start at 0.
-        assert(calc_results->first.front().size == 0);
-        assert(calc_results->first.front().fee == 0);
-        assert(calc_results->second.front().size == 0);
-        assert(calc_results->second.front().fee == 0);
-
-        CheckDiagramConcave(calc_results->first);
-        CheckDiagramConcave(calc_results->second);
+        // Feerates are monotonically decreasing.
+        FeeFrac first_sum;
+        for (size_t i = 0; i < calc_results->first.size(); ++i) {
+            first_sum += calc_results->first[i];
+            if (i) assert(!(calc_results->first[i - 1] << calc_results->first[i]));
+        }
+        FeeFrac second_sum;
+        for (size_t i = 0; i < calc_results->second.size(); ++i) {
+            second_sum += calc_results->second[i];
+            if (i) assert(!(calc_results->second[i - 1] << calc_results->second[i]));
+        }
 
-        CAmount replaced_fee{0};
-        int64_t replaced_size{0};
+        FeeFrac replaced;
         for (auto txiter : all_conflicts) {
-            replaced_fee += txiter->GetModifiedFee();
-            replaced_size += txiter->GetTxSize();
+            replaced.fee += txiter->GetModifiedFee();
+            replaced.size += txiter->GetTxSize();
         }
-        // The total fee of the new diagram should be the total fee of the old
-        // diagram - replaced_fee + replacement_fees
-        assert(calc_results->first.back().fee - replaced_fee + replacement_fees == calc_results->second.back().fee);
-        assert(calc_results->first.back().size - replaced_size + replacement_vsize == calc_results->second.back().size);
+        // The total fee & size of the new diagram minus replaced fee & size should be the total
+        // fee & size of the old diagram minus replacement fee & size.
+        assert((first_sum - replaced) == (second_sum - FeeFrac{replacement_fees, replacement_vsize}));
     }
 
     // If internals report error, wrapper should too
@@ -201,10 +191,12 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf)
          assert(err_tuple.value().first == DiagramCheckError::UNCALCULABLE);
     } else {
         // Diagram check succeeded
+        auto old_sum = std::accumulate(calc_results->first.begin(), calc_results->first.end(), FeeFrac{});
+        auto new_sum = std::accumulate(calc_results->second.begin(), calc_results->second.end(), FeeFrac{});
         if (!err_tuple.has_value()) {
             // New diagram's final fee should always match or exceed old diagram's
-            assert(calc_results->first.back().fee <= calc_results->second.back().fee);
-        } else if (calc_results->first.back().fee > calc_results->second.back().fee) {
+            assert(old_sum.fee <= new_sum.fee);
+        } else if (old_sum.fee > new_sum.fee) {
             // Or it failed, and if old diagram had higher fees, it should be a failure
             assert(err_tuple.value().first == DiagramCheckError::FAILURE);
         }
diff --git a/src/test/rbf_tests.cpp b/src/test/rbf_tests.cpp
index ed33969710..19e45c550a 100644
--- a/src/test/rbf_tests.cpp
+++ b/src/test/rbf_tests.cpp
@@ -426,21 +426,21 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup)
 
     // Replacement of size 1
     {
-        const auto replace_one{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/0, /*replacement_vsize=*/1, {entry_low}, {entry_low})};
+        const auto replace_one{pool.CalculateChunksForRBF(/*replacement_fees=*/0, /*replacement_vsize=*/1, {entry_low}, {entry_low})};
         BOOST_CHECK(replace_one.has_value());
-        std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee, low_size)};
-        BOOST_CHECK(replace_one->first == expected_old_diagram);
-        std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(0, 1)};
-        BOOST_CHECK(replace_one->second == expected_new_diagram);
+        std::vector<FeeFrac> expected_old_chunks{{low_fee, low_size}};
+        BOOST_CHECK(replace_one->first == expected_old_chunks);
+        std::vector<FeeFrac> expected_new_chunks{{0, 1}};
+        BOOST_CHECK(replace_one->second == expected_new_chunks);
     }
 
     // Non-zero replacement fee/size
     {
-        const auto replace_one_fee{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low})};
+        const auto replace_one_fee{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low})};
         BOOST_CHECK(replace_one_fee.has_value());
-        std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee, low_size)};
+        std::vector<FeeFrac> expected_old_diagram{{low_fee, low_size}};
         BOOST_CHECK(replace_one_fee->first == expected_old_diagram);
-        std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size)};
+        std::vector<FeeFrac> expected_new_diagram{{high_fee, low_size}};
         BOOST_CHECK(replace_one_fee->second == expected_new_diagram);
     }
 
@@ -451,22 +451,22 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup)
     const auto high_size = entry_high->GetTxSize();
 
     {
-        const auto replace_single_chunk{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low, entry_high})};
+        const auto replace_single_chunk{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_low}, {entry_low, entry_high})};
         BOOST_CHECK(replace_single_chunk.has_value());
-        std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee + high_fee, low_size + high_size)};
-        BOOST_CHECK(replace_single_chunk->first == expected_old_diagram);
-        std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size)};
-        BOOST_CHECK(replace_single_chunk->second == expected_new_diagram);
+        std::vector<FeeFrac> expected_old_chunks{{low_fee + high_fee, low_size + high_size}};
+        BOOST_CHECK(replace_single_chunk->first == expected_old_chunks);
+        std::vector<FeeFrac> expected_new_chunks{{high_fee, low_size}};
+        BOOST_CHECK(replace_single_chunk->second == expected_new_chunks);
     }
 
     // Conflict with the 2nd tx, resulting in new diagram with three entries
     {
-        const auto replace_cpfp_child{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high}, {entry_high})};
+        const auto replace_cpfp_child{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high}, {entry_high})};
         BOOST_CHECK(replace_cpfp_child.has_value());
-        std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(low_fee + high_fee, low_size + high_size)};
-        BOOST_CHECK(replace_cpfp_child->first == expected_old_diagram);
-        std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size), FeeFrac(low_fee + high_fee, low_size + low_size)};
-        BOOST_CHECK(replace_cpfp_child->second == expected_new_diagram);
+        std::vector<FeeFrac> expected_old_chunks{{low_fee + high_fee, low_size + high_size}};
+        BOOST_CHECK(replace_cpfp_child->first == expected_old_chunks);
+        std::vector<FeeFrac> expected_new_chunks{{high_fee, low_size}, {low_fee, low_size}};
+        BOOST_CHECK(replace_cpfp_child->second == expected_new_chunks);
     }
 
     // third transaction causes the topology check to fail
@@ -476,7 +476,7 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup)
     const auto normal_size = entry_normal->GetTxSize();
 
     {
-        const auto replace_too_large{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/normal_fee, /*replacement_vsize=*/normal_size, {entry_low}, {entry_low, entry_high, entry_normal})};
+        const auto replace_too_large{pool.CalculateChunksForRBF(/*replacement_fees=*/normal_fee, /*replacement_vsize=*/normal_size, {entry_low}, {entry_low, entry_high, entry_normal})};
         BOOST_CHECK(!replace_too_large.has_value());
         BOOST_CHECK_EQUAL(util::ErrorString(replace_too_large).original, strprintf("%s has 2 descendants, max 1 allowed", low_tx->GetHash().GetHex()));
     }
@@ -493,12 +493,12 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup)
     const auto low_size_2 = entry_low_2->GetTxSize();
 
     {
-        const auto replace_two_chunks_single_cluster{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high_2}, {entry_high_2, entry_low_2})};
+        const auto replace_two_chunks_single_cluster{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {entry_high_2}, {entry_high_2, entry_low_2})};
         BOOST_CHECK(replace_two_chunks_single_cluster.has_value());
-        std::vector<FeeFrac> expected_old_diagram{FeeFrac(0, 0), FeeFrac(high_fee, high_size_2), FeeFrac(low_fee + high_fee, low_size_2 + high_size_2)};
-        BOOST_CHECK(replace_two_chunks_single_cluster->first == expected_old_diagram);
-        std::vector<FeeFrac> expected_new_diagram{FeeFrac(0, 0), FeeFrac(high_fee, low_size_2)};
-        BOOST_CHECK(replace_two_chunks_single_cluster->second == expected_new_diagram);
+        std::vector<FeeFrac> expected_old_chunks{{high_fee, high_size_2}, {low_fee, low_size_2}};
+        BOOST_CHECK(replace_two_chunks_single_cluster->first == expected_old_chunks);
+        std::vector<FeeFrac> expected_new_chunks{{high_fee, low_size_2}};
+        BOOST_CHECK(replace_two_chunks_single_cluster->second == expected_new_chunks);
     }
 
     // You can have more than two direct conflicts if the there are multiple affected clusters, all of size 2 or less
@@ -515,10 +515,10 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup)
     const auto conflict_3_entry = pool.GetIter(conflict_3->GetHash()).value();
 
     {
-        const auto replace_multiple_clusters{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry})};
+        const auto replace_multiple_clusters{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry})};
         BOOST_CHECK(replace_multiple_clusters.has_value());
-        BOOST_CHECK(replace_multiple_clusters->first.size() == 4);
-        BOOST_CHECK(replace_multiple_clusters->second.size() == 2);
+        BOOST_CHECK(replace_multiple_clusters->first.size() == 3);
+        BOOST_CHECK(replace_multiple_clusters->second.size() == 1);
     }
 
     // Add a child transaction to conflict_1 and make it cluster size 2, two chunks due to same feerate
@@ -527,11 +527,11 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup)
     const auto conflict_1_child_entry = pool.GetIter(conflict_1_child->GetHash()).value();
 
     {
-        const auto replace_multiple_clusters_2{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry})};
+        const auto replace_multiple_clusters_2{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry})};
 
         BOOST_CHECK(replace_multiple_clusters_2.has_value());
-        BOOST_CHECK(replace_multiple_clusters_2->first.size() == 5);
-        BOOST_CHECK(replace_multiple_clusters_2->second.size() == 2);
+        BOOST_CHECK(replace_multiple_clusters_2->first.size() == 4);
+        BOOST_CHECK(replace_multiple_clusters_2->second.size() == 1);
     }
 
     // Add another descendant to conflict_1, making the cluster size > 2 should fail at this point.
@@ -540,86 +540,86 @@ BOOST_FIXTURE_TEST_CASE(calc_feerate_diagram_rbf, TestChain100Setup)
     const auto conflict_1_grand_child_entry = pool.GetIter(conflict_1_child->GetHash()).value();
 
     {
-        const auto replace_cluster_size_3{pool.CalculateFeerateDiagramsForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry, conflict_1_grand_child_entry})};
+        const auto replace_cluster_size_3{pool.CalculateChunksForRBF(/*replacement_fees=*/high_fee, /*replacement_vsize=*/low_size, {conflict_1_entry, conflict_2_entry, conflict_3_entry}, {conflict_1_entry, conflict_2_entry, conflict_3_entry, conflict_1_child_entry, conflict_1_grand_child_entry})};
 
         BOOST_CHECK(!replace_cluster_size_3.has_value());
         BOOST_CHECK_EQUAL(util::ErrorString(replace_cluster_size_3).original, strprintf("%s has 2 descendants, max 1 allowed", conflict_1->GetHash().GetHex()));
     }
 }
 
-BOOST_AUTO_TEST_CASE(feerate_diagram_utilities)
+BOOST_AUTO_TEST_CASE(feerate_chunks_utilities)
 {
-    // Sanity check the correctness of the feerate diagram comparison.
+    // Sanity check the correctness of the feerate chunks comparison.
 
     // A strictly better case.
-    std::vector<FeeFrac> old_diagram{{FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}}};
-    std::vector<FeeFrac> new_diagram{{FeeFrac{0, 0}, FeeFrac{1000, 300}, FeeFrac{1050, 400}}};
+    std::vector<FeeFrac> old_chunks{{{950, 300}, {100, 100}}};
+    std::vector<FeeFrac> new_chunks{{{1000, 300}, {50, 100}}};
 
-    BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram)));
-    BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram)));
+    BOOST_CHECK(std::is_lt(CompareChunks(old_chunks, new_chunks)));
+    BOOST_CHECK(std::is_gt(CompareChunks(new_chunks, old_chunks)));
 
     // Incomparable diagrams
-    old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}};
-    new_diagram = {FeeFrac{0, 0}, FeeFrac{1000, 300}, FeeFrac{1000, 400}};
+    old_chunks = {{950, 300}, {100, 100}};
+    new_chunks = {{1000, 300}, {0, 100}};
 
-    BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered);
-    BOOST_CHECK(CompareFeerateDiagram(new_diagram, old_diagram) == std::partial_ordering::unordered);
+    BOOST_CHECK(CompareChunks(old_chunks, new_chunks) == std::partial_ordering::unordered);
+    BOOST_CHECK(CompareChunks(new_chunks, old_chunks) == std::partial_ordering::unordered);
 
     // Strictly better but smaller size.
-    old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}};
-    new_diagram = {FeeFrac{0, 0}, FeeFrac{1100, 300}};
+    old_chunks = {{950, 300}, {100, 100}};
+    new_chunks = {{1100, 300}};
 
-    BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram)));
-    BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram)));
+    BOOST_CHECK(std::is_lt(CompareChunks(old_chunks, new_chunks)));
+    BOOST_CHECK(std::is_gt(CompareChunks(new_chunks, old_chunks)));
 
     // New diagram is strictly better due to the first chunk, even though
     // second chunk contributes no fees
-    old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}};
-    new_diagram = {FeeFrac{0, 0}, FeeFrac{1100, 100}, FeeFrac{1100, 200}};
+    old_chunks = {{950, 300}, {100, 100}};
+    new_chunks = {{1100, 100}, {0, 100}};
 
-    BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram)));
-    BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram)));
+    BOOST_CHECK(std::is_lt(CompareChunks(old_chunks, new_chunks)));
+    BOOST_CHECK(std::is_gt(CompareChunks(new_chunks, old_chunks)));
 
     // Feerate of first new chunk is better with, but second chunk is worse
-    old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}};
-    new_diagram = {FeeFrac{0, 0}, FeeFrac{750, 100}, FeeFrac{999, 350}, FeeFrac{1150, 1000}};
+    old_chunks = {{950, 300}, {100, 100}};
+    new_chunks = {{750, 100}, {249, 250}, {151, 650}};
 
-    BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered);
-    BOOST_CHECK(CompareFeerateDiagram(new_diagram, old_diagram) == std::partial_ordering::unordered);
+    BOOST_CHECK(CompareChunks(old_chunks, new_chunks) == std::partial_ordering::unordered);
+    BOOST_CHECK(CompareChunks(new_chunks, old_chunks) == std::partial_ordering::unordered);
 
     // If we make the second chunk slightly better, the new diagram now wins.
-    old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}};
-    new_diagram = {FeeFrac{0, 0}, FeeFrac{750, 100}, FeeFrac{1000, 350}, FeeFrac{1150, 500}};
+    old_chunks = {{950, 300}, {100, 100}};
+    new_chunks = {{750, 100}, {250, 250}, {150, 150}};
 
-    BOOST_CHECK(std::is_lt(CompareFeerateDiagram(old_diagram, new_diagram)));
-    BOOST_CHECK(std::is_gt(CompareFeerateDiagram(new_diagram, old_diagram)));
+    BOOST_CHECK(std::is_lt(CompareChunks(old_chunks, new_chunks)));
+    BOOST_CHECK(std::is_gt(CompareChunks(new_chunks, old_chunks)));
 
     // Identical diagrams, cannot be strictly better
-    old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}};
-    new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}};
+    old_chunks = {{950, 300}, {100, 100}};
+    new_chunks = {{950, 300}, {100, 100}};
 
-    BOOST_CHECK(std::is_eq(CompareFeerateDiagram(old_diagram, new_diagram)));
-    BOOST_CHECK(std::is_eq(CompareFeerateDiagram(new_diagram, old_diagram)));
+    BOOST_CHECK(std::is_eq(CompareChunks(old_chunks, new_chunks)));
+    BOOST_CHECK(std::is_eq(CompareChunks(new_chunks, old_chunks)));
 
     // Same aggregate fee, but different total size (trigger single tail fee check step)
-    old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 399}};
-    new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}};
+    old_chunks = {{950, 300}, {100, 99}};
+    new_chunks = {{950, 300}, {100, 100}};
 
     // No change in evaluation when tail check needed.
-    BOOST_CHECK(std::is_gt(CompareFeerateDiagram(old_diagram, new_diagram)));
-    BOOST_CHECK(std::is_lt(CompareFeerateDiagram(new_diagram, old_diagram)));
+    BOOST_CHECK(std::is_gt(CompareChunks(old_chunks, new_chunks)));
+    BOOST_CHECK(std::is_lt(CompareChunks(new_chunks, old_chunks)));
 
     // Trigger multiple tail fee check steps
-    old_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 399}};
-    new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}, FeeFrac{1050, 401}, FeeFrac{1050, 402}};
+    old_chunks = {{950, 300}, {100, 99}};
+    new_chunks = {{950, 300}, {100, 100}, {0, 1}, {0, 1}};
 
-    BOOST_CHECK(std::is_gt(CompareFeerateDiagram(old_diagram, new_diagram)));
-    BOOST_CHECK(std::is_lt(CompareFeerateDiagram(new_diagram, old_diagram)));
+    BOOST_CHECK(std::is_gt(CompareChunks(old_chunks, new_chunks)));
+    BOOST_CHECK(std::is_lt(CompareChunks(new_chunks, old_chunks)));
 
     // Multiple tail fee check steps, unordered result
-    new_diagram = {FeeFrac{0, 0}, FeeFrac{950, 300}, FeeFrac{1050, 400}, FeeFrac{1050, 401}, FeeFrac{1050, 402}, FeeFrac{1051, 403}};
-    BOOST_CHECK(CompareFeerateDiagram(old_diagram, new_diagram) == std::partial_ordering::unordered);
-    BOOST_CHECK(CompareFeerateDiagram(new_diagram, old_diagram) == std::partial_ordering::unordered);
+    new_chunks = {{950, 300}, {100, 100}, {0, 1}, {0, 1}, {1, 1}};
+    BOOST_CHECK(CompareChunks(old_chunks, new_chunks) == std::partial_ordering::unordered);
+    BOOST_CHECK(CompareChunks(new_chunks, old_chunks) == std::partial_ordering::unordered);
 }
 
 BOOST_AUTO_TEST_SUITE_END()