Merge bitcoin/bitcoin#34243: doc: validation: fix PackageMempoolChecks incorrect comment

7fc465ece8 doc: fix incorrect description of `PackageMempoolChecks` (ismaelsadeeq)
1412b779ad refactor: execute `PackageMempoolChecks` during package rbf only (ismaelsadeeq)

Pull request description:

  This is a simple PR that fixes the incorrect description of what is done in `PackageMempoolChecks`

  >  // Enforce package mempool ancestor/descendant limits (distinct from individual
  > // ancestor/descendant limits done in PreChecks) and run Package RBF checks.

  After cluster mempool, we no longer enforce ancestor/descendant limits in both `PreChecks` and  `PackageMempoolChecks`; instead, cluster limit is enforced in `PackageMempoolChecks`.
   This PR fixes the incorrect comment  by;
     - Making it clear why it is necessary to have two calls of  `CheckMempoolPolicyLimts` in both `PackageMempoolChecks` and after in `AcceptMultipleTransactionsInternal` by executing `PackageMempoolChecks` only during package RBF only. No need to jump into the next subroutine when there is no conflict.
     - Renames `PackageMempoolChecks` to `PackageRBFChecks`; the method name is self-explanatory now, hence no need for a description comment.

ACKs for top commit:
  yashbhutwala:
    ACK 7fc465ece8
  instagibbs:
    ACK 7fc465ece8
  glozow:
    utACK 7fc465ece8

Tree-SHA512: 38655f9d05be54cadd224fad376da9871a85efc7801306b58d4f51aee658036cdce2ab406143a3439d7211fc9bb0fc86bd330852e8926d79660944872b8fae8d

src/validation.cpp

@@ -672,12 +672,10 @@ private:
     // Run checks for mempool replace-by-fee, only used in AcceptSingleTransaction.
     bool ReplacementChecks(Workspace& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs);
 
-    // Enforce package mempool ancestor/descendant limits (distinct from individual
+    bool PackageRBFChecks(const std::vector<CTransactionRef>& txns,
-    // ancestor/descendant limits done in PreChecks) and run Package RBF checks.
+                          std::vector<Workspace>& workspaces,
-    bool PackageMempoolChecks(const std::vector<CTransactionRef>& txns,
+                          int64_t total_vsize,
-                              std::vector<Workspace>& workspaces,
+                          PackageValidationState& package_state) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs);
-                              int64_t total_vsize,
-                              PackageValidationState& package_state) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs);
 
     // Run the script checks using our policy flags. As this can be slow, we should
     // only invoke this on transactions that have otherwise passed policy checks.
@@ -1035,10 +1033,10 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws)
     return true;
 }
 
-bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txns,
+bool MemPoolAccept::PackageRBFChecks(const std::vector<CTransactionRef>& txns,
-                                         std::vector<Workspace>& workspaces,
+                                     std::vector<Workspace>& workspaces,
-                                         const int64_t total_vsize,
+                                     const int64_t total_vsize,
-                                         PackageValidationState& package_state)
+                                     PackageValidationState& package_state)
 {
     AssertLockHeld(cs_main);
     AssertLockHeld(m_pool.cs);
@@ -1048,9 +1046,6 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txn
 
     assert(txns.size() == workspaces.size());
 
-    // No conflicts means we're finished. Further checks are all RBF-only.
-    if (!m_subpackage.m_rbf) return true;
-
     // We're in package RBF context; replacement proposal must be size 2
     if (workspaces.size() != 2 || !Assume(IsChildWithParents(txns))) {
         return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: package must be 1-parent-1-child");
@@ -1473,7 +1468,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactionsInternal(con
         // needed by another transaction in the package. We also need to make sure that no package
         // tx replaces (or replaces the ancestor of) the parent of another package tx. As long as we
         // check these two things, we don't need to track the coins spent.
-        // If a package tx conflicts with a mempool tx, PackageMempoolChecks() ensures later that any package RBF attempt
+        // If a package tx conflicts with a mempool tx, PackageRBFChecks() ensures later that any package RBF attempt
         // has *no* in-mempool ancestors, so we don't have to worry about subsequent transactions in
         // same package spending the same in-mempool outpoints. This needs to be revisited for general
         // package RBF.
@@ -1516,7 +1511,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactionsInternal(con
     }
 
     // Apply package mempool RBF checks.
-    if (!PackageMempoolChecks(txns, workspaces, m_subpackage.m_total_vsize, package_state)) {
+    if (m_subpackage.m_rbf && !PackageRBFChecks(txns, workspaces, m_subpackage.m_total_vsize, package_state)) {
         return PackageMempoolAcceptResult(package_state, std::move(results));
     }