doc: validation: fix PackageMempoolChecks incorrect comment #34243

1. 
   ismaelsadeeq commented at 7:25 pm on January 9, 2026: member

   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.
2. refactor: execute `PackageMempoolChecks` during package rbf only

   - No need to jump into the next subroutine when there is no conflict.
   
   - This makes it clear why it is necessary to have two calls of
     CheckMempoolPolicyLimts in both PackageMempoolChecks and after in
     AcceptMultipleTransactionsInternal, there is a possibilty that we
     we want to accept multiple transaction but they are not conflicting
     with any in-mempool transaction, in that case also we want to check
     that they do not bust the cluster limits.

   1412b779ad
3. DrahtBot added the label Docs on Jan 9, 2026
4. 
   DrahtBot commented at 7:25 pm on January 9, 2026: contributor

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

   Code Coverage & Benchmarks

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

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	glozow, yashbhutwala, instagibbs

   If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

5. in src/validation.cpp:1516 in 1412b779ad outdated

   1512@@ -1516,7 +1513,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactionsInternal(con
   1513     }
   1514 
   1515     // Apply package mempool RBF checks.
   1516-    if (!PackageMempoolChecks(txns, workspaces, m_subpackage.m_total_vsize, package_state)) {
   1517+    if (m_subpackage.m_rbf && !PackageMempoolChecks(txns, workspaces, m_subpackage.m_total_vsize, package_state)) {
   

   ---

   ---

   

   instagibbs commented at 7:31 pm on January 9, 2026:

   do we want to rename it to make it more obvious since that’s all we’re doing? PackageRBFChecks?

   ---

   

   ismaelsadeeq commented at 7:41 pm on January 9, 2026:

   I thought about it, but could not come up with a better name. I take your suggestion. pushed in 2e1dff1b1d10f4dca3ba7fefaa32e0db3c529b80
6. DrahtBot added the label CI failed on Jan 9, 2026
7. DrahtBot removed the label CI failed on Jan 9, 2026
8. in src/validation.cpp:675 in c24ab443ce outdated

   671@@ -672,8 +672,7 @@ class MemPoolAccept
   672     // Run checks for mempool replace-by-fee, only used in AcceptSingleTransaction.
   673     bool ReplacementChecks(Workspace& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs);
   674 
   675-    // Enforce package mempool ancestor/descendant limits (distinct from individual
   676-    // ancestor/descendant limits done in PreChecks) and run Package RBF checks.
   677+    // Enforce package mempool cluster limits and run Package RBF checks.
   

   ---

   ---

   

   glozow commented at 8:42 pm on January 12, 2026:

   Should probably just be “run Package RBF checks” or nothing (since the name is self-explanatory now)

   ---

   

   ismaelsadeeq commented at 5:34 pm on January 13, 2026:

   You are right, fixed and updated the PR description; however, it’s a bit awkward. I added a comment and deleted it in the next commit Let me know whether it is better to just squash everything into a single commit.

   I just did it this way for convenience and the flow of commit history.

   ---

   

   glozow commented at 5:57 pm on January 13, 2026:

   Thanks, I think it’s better squashed

   ---

   

   ismaelsadeeq commented at 6:55 pm on January 13, 2026:

   Done.
9. ismaelsadeeq force-pushed on Jan 13, 2026
10. doc: fix incorrect description of `PackageMempoolChecks`

    - We no longer enforce ancestor/descendant count limit
      in both PreChecks and PackageMempoolChecks.
    
    - This commit fixes the incorrect comment by just renaming
      `PackageMempoolChecks` to `PackageRBFChecks`
    
    - The method name is self explanatory now; hence no need
      for a description comment.

    7fc465ece8
11. ismaelsadeeq force-pushed on Jan 13, 2026
12. 
    glozow commented at 1:15 am on January 14, 2026: member

    utACK 7fc465ece88284c79728cacbc1d4c2fe63c60e1a
13. fanquake requested review from instagibbs on Jan 14, 2026
14. 
    yashbhutwala commented at 3:33 pm on January 14, 2026: none

    ACK 7fc465ece88284c79728cacbc1d4c2fe63c60e1a

    Code review and tested locally:

    • Verified the refactoring is behavior-preserving: moving the m_rbf check from inside PackageMempoolChecks to the call site preserves the same control flow
    • Built successfully and ran txpackage_tests (including package_rbf_tests), rbf_tests, and mempool_tests - all pass
    • The rename to PackageRBFChecks is appropriate since this function now exclusively handles package RBF logic (cluster limits are checked separately in CheckMemPoolPolicyLimits)
    • Removing the outdated comment and making the function name self-explanatory improves code clarity

    Minor observation: The two commits could potentially be squashed since the first commit references PackageMempoolChecks in its title while the second commit renames it, but keeping them separate also provides a clear logical progression. Either way works.

15. 
    instagibbs commented at 4:18 pm on January 14, 2026: member

    ACK 7fc465ece88284c79728cacbc1d4c2fe63c60e1a
16. fanquake merged this on Jan 14, 2026
17. fanquake closed this on Jan 14, 2026

    ---

18. ismaelsadeeq deleted the branch on Jan 15, 2026

Contributors
ismaelsadeeq DrahtBot instagibbs glozow yashbhutwala

Review Requested
instagibbs

Labels
Docs