Merge bitcoin/bitcoin#29724: 29242 Diagram check followups

ee1b9b2 CalculateFeerateDiagramsForRBF: update misleading description of old diagram contents (Greg Sanders)
a9d42b9 CompareFeerateDiagram: short-circuit comparison when detected as incomparable (Greg Sanders)
cebcced remove erroneous CompareFeerateDiagram comment about slope (Greg Sanders)
a0376e1 unit test: clarify unstated assumption for calc_feerate_diagram_rbf chunking (Greg Sanders)
890cb01 s/effected/affected/ (Greg Sanders)
d9391ec CalculateFeerateDiagramsForRBF: remove size tie-breaking from chunking conflicts (Greg Sanders)
b684d82 fuzz: Add more invariant checks for package_rbf (Greg Sanders)
2a3ada8 fuzz: finer grained ImprovesFeerateDiagram check on error result (Greg Sanders)
c377ae9 unit test: improve ImprovesFeerateDiagram coverage with one less vb case (Greg Sanders)
d2bf923 unit test: make calc_feerate_diagram_rbf less brittle (Greg Sanders)
defe023 fuzz: add PrioritiseTransaction coverage in diagram checks (Greg Sanders)
216d5ff unit test: add coverage showing priority affects diagram check results (Greg Sanders)
a80d809 unit test: add CheckConflictTopology case for not the only child (Greg Sanders)
69bd18c unit test: check tx4 conflict error message (Greg Sanders)
c0c37f0 unit test: have CompareFeerateDiagram tested with diagrams both ways (Greg Sanders)
b62e2c0 ImprovesFeerateDiagram: Spelling fix and removal of unused diagram vectors (Greg Sanders)
bb42402 doc: fix comment about non-existing CompareFeeFrac (Greg Sanders)

Pull request description:

  Follow-ups to bitcoin/bitcoin#29242

ACKs for top commit:
  glozow:
    ACK ee1b9b2, reviewed the changes and package_rbf fuzzer seems to run fine
  murchandamus:
    crACK ee1b9b2
  ismaelsadeeq:
    Code review ACK ee1b9b2
  willcl-ark:
    ACK ee1b9b2

Tree-SHA512: 8399fe12064fb49b0e4c73258968b57be1d9c2e35701b2d3b0bb67e2e4052e44216358238f92508e4697d0fb6176518d5b885474054d3deda242f669e99262a7

Author: achow101

src/policy/rbf.cpp

@@ -190,9 +190,7 @@ std::optional<std::pair<DiagramCheckError, std::string>> ImprovesFeerateDiagram(
                                                 CAmount replacement_fees,
                                                 int64_t replacement_vsize)
 {
-    // Require that the replacement strictly improve the mempool's feerate diagram.
-    std::vector<FeeFrac> old_diagram, new_diagram;
-
+    // Require that the replacement strictly improves the mempool's feerate diagram.
     const auto diagram_results{pool.CalculateFeerateDiagramsForRBF(replacement_fees, replacement_vsize, direct_conflicts, all_conflicts)};
 
     if (!diagram_results.has_value()) {

src/test/fuzz/rbf.cpp

@@ -127,6 +127,10 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf)
         }
         mempool_txs.emplace_back(*child);
         pool.addUnchecked(ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back()));
+
+        if (fuzzed_data_provider.ConsumeBool()) {
+            pool.PrioritiseTransaction(mempool_txs.back().GetHash().ToUint256(), fuzzed_data_provider.ConsumeIntegralInRange<int32_t>(-100000, 100000));
+        }
     }
 
     // Pick some transactions at random to be the direct conflicts
@@ -174,5 +178,16 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf)
 
     // If internals report error, wrapper should too
     auto err_tuple{ImprovesFeerateDiagram(pool, direct_conflicts, all_conflicts, replacement_fees, replacement_vsize)};
-    if (!calc_results.has_value()) assert(err_tuple.has_value());
+    if (!calc_results.has_value()) {
+         assert(err_tuple.value().first == DiagramCheckError::UNCALCULABLE);
+    } else {
+        // Diagram check succeeded
+        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) {
+            // Or it failed, and if old diagram had higher fees, it should be a failure
+            assert(err_tuple.value().first == DiagramCheckError::FAILURE);
+        }
+    }
 }