Merge bitcoin/bitcoin#22855: RBF move 3/3: move followups + improve RBF documentation

0ef08f8bed537435f3f9db1e38b7d6f3551fe830 add missing includes in policy/rbf (glozow)
c6abeb76fbb877f3f16d699c73a1828c7da2e6d1 make MAX_BIP125_RBF_SEQUENCE constexpr (glozow)
3cf46f6055f7cd2e5da81e0d29cafc51ad4aafba [doc] improve RBF documentation (glozow)
c78eb8651b0949fefcafb22940512f4ef98d3358 [policy/refactor] pass in relay fee instead of using global (glozow)

Pull request description:

  Followups to #22675 and documentation-only changes intended to clarify the code/logic concerning mempool Replace-by-Fee.

ACKs for top commit:
  jnewbery:
    utACK 0ef08f8bed537435f3f9db1e38b7d6f3551fe830
  fanquake:
    ACK 0ef08f8bed537435f3f9db1e38b7d6f3551fe830

Tree-SHA512: 6797ae758beca0c9673cb00ce85da48e9a4ac5cb5100074ca93e004cdb31d24d91a1a7721b57fc2f619addfeb4950d8caf45fee0f5b7528defbbd121eb4d271f

1 files changed, 14 insertions, 10 deletions

diff --git a/src/validation.cpp b/src/validation.cpp
index 2a66d96f22..eeecfb147f 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -772,39 +772,43 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
     // pathological case by making sure setConflicts and setAncestors don't
     // intersect.
     if (const auto err_string{EntriesAndTxidsDisjoint(setAncestors, setConflicts, hash)}) {
+        // We classify this as a consensus error because a transaction depending on something it
+        // conflicts with would be inconsistent.
         return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-spends-conflicting-tx", *err_string);
     }
 
 
-    // If we don't hold the lock allConflicting might be incomplete; the
-    // subsequent RemoveStaged() and addUnchecked() calls don't guarantee
-    // mempool consistency for us.
     fReplacementTransaction = setConflicts.size();
-    if (fReplacementTransaction)
-    {
+    if (fReplacementTransaction) {
         CFeeRate newFeeRate(nModifiedFees, nSize);
+        // It's possible that the replacement pays more fees than its direct conflicts but not more
+        // than all conflicts (i.e. the direct conflicts have high-fee descendants). However, if the
+        // replacement doesn't pay more fees than its direct conflicts, then we can be sure it's not
+        // more economically rational to mine. Before we go digging through the mempool for all
+        // transactions that would need to be removed (direct conflicts and all descendants), check
+        // that the replacement transaction pays more than its direct conflicts.
         if (const auto err_string{PaysMoreThanConflicts(setIterConflicting, newFeeRate, hash)}) {
             return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string);
         }
 
-        // Calculate all conflicting entries and enforce Rule #5.
+        // Calculate all conflicting entries and enforce BIP125 Rule #5.
         if (const auto err_string{GetEntriesForConflicts(tx, m_pool, setIterConflicting, allConflicting)}) {
             return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY,
                                  "too many potential replacements", *err_string);
         }
-        // Enforce Rule #2.
+        // Enforce BIP125 Rule #2.
         if (const auto err_string{HasNoNewUnconfirmed(tx, m_pool, setIterConflicting)}) {
             return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY,
                                  "replacement-adds-unconfirmed", *err_string);
         }
 
-        // Check if it's economically rational to mine this transaction rather
-        // than the ones it replaces. Enforce Rules #3 and #4.
+        // Check if it's economically rational to mine this transaction rather than the ones it
+        // replaces and pays for its own relay fees. Enforce BIP125 Rules #3 and #4.
         for (CTxMemPool::txiter it : allConflicting) {
             nConflictingFees += it->GetModifiedFee();
             nConflictingSize += it->GetTxSize();
         }
-        if (const auto err_string{PaysForRBF(nConflictingFees, nModifiedFees, nSize, hash)}) {
+        if (const auto err_string{PaysForRBF(nConflictingFees, nModifiedFees, nSize, ::incrementalRelayFee, hash)}) {
             return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string);
         }
     }