rpc: add testsubmitpackage for 1p1c test submissions #35286

1. 
   instagibbs commented at 6:16 PM on May 13, 2026: member

   A test-only companion to submitpackage that validates a strict 1p1c [parent, child] bundle without mutating the mempool. Intended as a wallet pre-flight check for CPFP fee-bumping packages.

   Enables rbf and sibling eviction evaluation, just like submitpackage.

   Diverging behaviors from submitpackage:

   Hard rejects packages that aren't exactly 2 txs in a child-with-parents topology, and parents that pass individual acceptance (caller should use testsubmitpackage on parent alone for that).

   ---

   To do this "properly" without rejecting the self-sufficient parents (and not giving surprising results that don't match reality), we would need fairly extensive changes to mempool acceptance logic, more book-keeping to unroll test-accepted subpackage state (while risking mutating the mempool in brittle ways), and possibly other changes.

   This is a partial addressing of #32160, hopefully addressing most of the missing functionality people are requesting.

2. DrahtBot added the label RPC/REST/ZMQ on May 13, 2026
3. 
   DrahtBot commented at 6:16 PM on May 13, 2026: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--006a51241073e994b41acfe9ec718e94-->

   Code Coverage & Benchmarks

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

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Concept ACK	ismaelsadeeq

   If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #32800 (rpc: Distinguish between vsize and sigop adjusted mempool vsize by musaHaruna)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

   <!--5faf32d7da4f0f540f40219e4f7537a3-->

   LLM Linter (✨ experimental)

   Possible places where comparison-specific test macros should replace generic comparisons:

   • [test/functional/rpc_testsubmitpackage.py] assert parent_A.wtxid_hex != parent_B.wtxid_hex, ( -> assert_not_equal(parent_A.wtxid_hex, parent_B.wtxid_hex)
   • [test/functional/rpc_testsubmitpackage.py] assert set(eff) == {parent["wtxid"], child["wtxid"]}, eff -> assert_equal(set(eff), {parent["wtxid"], child["wtxid"]})
   • [test/functional/rpc_testsubmitpackage.py] assert set(child_entry["fees"]["effective-includes"]) == {parent["wtxid"], child["wtxid"]} -> assert_equal(set(child_entry["fees"]["effective-includes"]), {parent["wtxid"], child["wtxid"]})

   ^{2026-05-13 18:48:06}

4. 
   instagibbs commented at 6:17 PM on May 13, 2026: member

   cc @ajtowns @ismaelsadeeq as discussed, let's focus on concept / approach ACKs first, name is clearly up for debate, etc etc

5. rpc: add testsubmitpackage for 1p1c test submissions

   A test-only companion to submitpackage that validates a strict 1p1c
   [parent, child] bundle without mutating the mempool. Intended as a
   wallet pre-flight check for CPFP fee-bumping packages.
   
   Enables rbf and sibling eviction evaluation, just like
   submitblock.
   
   Diverging behaviors from submitpackage:
   
   Hard rejects packages that aren't exactly 2 txs in a child-with-parents
   topology, and parents that pass individual acceptance (caller should
   use testsubmitpackage on parent alone for that).

   a723ef365c
6. instagibbs force-pushed on May 13, 2026
7. DrahtBot added the label CI failed on May 13, 2026
8. 
   DrahtBot commented at 6:49 PM on May 13, 2026: contributor

   <!--85328a0da195eb286784d51f73fa0af9-->

   🚧 At least one of the CI tasks failed. _{Task Windows native, fuzz, VS: https://github.com/bitcoin/bitcoin/actions/runs/25817910490/job/75851154109} _{LLM reason (✨ experimental): Fuzz RPC tests failed because the RPC command testsubmitpackage was not registered in RPC_COMMANDS_SAFE_FOR_FUZZING/RPC_COMMANDS_NOT_SAFE_FOR_FUZZING (per src/test/fuzz/rpc.cpp).}

   <details><summary>Hints</summary>

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

   </details>

9. DrahtBot removed the label CI failed on May 13, 2026
10. sedited requested review from stickies-v on May 27, 2026
11. 
    ismaelsadeeq commented at 8:23 AM on May 29, 2026: member

    Concept ACK, users normally test run submissions to see whether they will get through; so having testsubmitpackage rpc to test run 1p1c package is very useful.

    I will review the approach and correctness.

12. in src/validation.cpp:524 in a723ef365c

     524 |                              /*bypass_limits=*/ false,
     525 |                              /*coins_to_uncache=*/ coins_to_uncache,
     526 | -                            /*test_accept=*/ false,
     527 | +                            /*test_accept=*/ test_accept,
     528 |                              /*allow_replacement=*/ true,
     529 | -                            /*allow_sibling_eviction=*/ false,
    

    ---

    ---

    

    ismaelsadeeq commented at 12:56 PM on May 29, 2026:

    In a723ef365c88c3bcbf1f5c1ff00137d62651d6aa This is weird to read? why is allow_sibling_eviction tied to test_accept?

13. in src/validation.h:284 in a723ef365c

     280 | @@ -281,12 +281,16 @@ MempoolAcceptResult AcceptToMemoryPool(Chainstate& active_chainstate, const CTra
     281 |  * @param[in]    test_accept         When true, run validation checks but don't submit to mempool.
     282 |  * @param[in]    client_maxfeerate    If exceeded by an individual transaction, rest of (sub)package evaluation is aborted.
     283 |  *                                   Only for sanity checks against local submission of transactions.
     284 | +* @param[in]    test_package_feerates If true (only honored when test_accept is true), use child-with-parents
    

    ---

    ---

    

    ismaelsadeeq commented at 1:39 PM on May 29, 2026:

    In a723ef365c88c3bcbf1f5c1ff00137d62651d6aa

    test_package_feerates is a bit hard to read?

    Can you use a clearer name for this, maybe test_accept_as_package?

14. in src/validation.cpp:1390 in a723ef365c

    1384 | @@ -1383,6 +1385,13 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransactionInternal(const CTransa
    1385 |      const CFeeRate effective_feerate{ws.m_modified_fees, static_cast<int32_t>(ws.m_vsize)};
    1386 |      // Tx was accepted, but not added
    1387 |      if (args.m_test_accept) {
    1388 | +        // Populate the would-be-replaced txs from the changeset for surfacing through the result.
    1389 | +        // In real submission FinalizeSubpackage() does this; we exit before it runs.
    1390 | +        if (Assume(m_subpackage.m_changeset)) {
    

    ---

    ---

    

    ismaelsadeeq commented at 1:56 PM on May 29, 2026:

    In a723ef365c88c3bcbf1f5c1ff00137d62651d6aa

    • The loop that copies staged removals into replaced transactions is duplicated at both test_accept sites. A helper like CollectStagedRemovals() would dedup things
    • The two sites guard the changeset access inconsistently one uses if (test_accept && Assume(changeset)), the other nests if (test_accept) { if (Assume(changeset)). Would be nice to make them consistent ( a helper would solve this too since the two code paths are identical).
15. in src/validation.cpp:1844 in a723ef365c

    1840 |      std::vector<COutPoint> coins_to_uncache;
    1841 |      const CChainParams& chainparams = active_chainstate.m_chainman.GetParams();
    1842 |      auto result = [&]() EXCLUSIVE_LOCKS_REQUIRED(cs_main) {
    1843 |          AssertLockHeld(cs_main);
    1844 |          if (test_accept) {
    1845 | +            if (test_package_feerates) {
    

    ---

    ---

    

    ismaelsadeeq commented at 2:22 PM on May 29, 2026:

    In a723ef365c88c3bcbf1f5c1ff00137d62651d6aa

    I think this is a good time to split the individual-evaluation dry-run path out of ProcessNewPackage into its own function. This removes the test_package_feerates parameter, and the branching ProcessNewPackage now always goes through AcceptPackage.

16. in test/functional/rpc_testsubmitpackage.py:71 in a723ef365c

      66 | +
      67 | +        self.test_input_validation()
      68 | +        self.test_topology_rejected()
      69 | +        self.test_parent_passes_individually_rejected()
      70 | +        self.test_basic_1p1c_cpfp()
      71 | +        self.test_no_mempool_mutation_on_success()
    

    ---

    ---

    

    ismaelsadeeq commented at 2:33 PM on May 29, 2026:

    In a723ef365c88c3bcbf1f5c1ff00137d62651d6aa

    Seems redundant to have this independent. Can we fold test_no_mempool_mutation_on_success into test_basic_1p1c_cpfp they build the same 1p1c bundle; the mempool-unchanged check belongs alongside the basic success assertions rather than in a separate test?

17. in test/functional/rpc_testsubmitpackage.py:259 in a723ef365c

     254 | +
     255 | +        # With maxfeerate=0 (disabled), the same package goes through.
     256 | +        result_ok = node.testsubmitpackage([parent["hex"], child["hex"]], maxfeerate=0)
     257 | +        assert_equal(result_ok["package_msg"], "success")
     258 | +
     259 | +        self.generate(self.wallet, 1)
    

    ---

    ---

    

    ismaelsadeeq commented at 2:44 PM on May 29, 2026:

    In a723ef365c88c3bcbf1f5c1ff00137d62651d6aa

    Side note? It seems like a hack to always generate a block when we want to remove a transaction or restart the node, which is much heavier. I think it's part of the reason why the tests are slow. It is worth having evict from mempool rpc for this case and others, which could eliminate this.

    Another way to do this is to also use unit tests, and remove them directly.

18. 
    ismaelsadeeq commented at 2:54 PM on May 29, 2026: member

    This is simple to reason about. I suggest some improvements that will make the code easier to reason about. Also, I think there are still opportunities to dedup things in rpc/mempool.cpp shared code of testsubmitpackage and submitpackage, in the code args collection, help text and the logic.

19. 
    instagibbs commented at 3:02 PM on May 29, 2026: member

    @ismaelsadeeq Interestingly I haven't had a lot of positive feedback yet on this PR. In the LN world, for example, the parent may end up having fees >=minfee, due to trimmed htlcs. Many of these projects don't even use test RPCs.

    I think it might make sense to look for more buy-in from prospective users before iterating further.

Review Requested
stickies-v