init: add -test=pause_load_mempool, test mempool save before loaded #34970

1. 
   Sjors commented at 7:33 pm on March 31, 2026: member

   A new -test=pause_load_mempool startup argument that lets the test framework pause node initialization right before loading the mempool.

   When set, the node checks for the existence of a pause_load_mempool file and waits until that file is removed.

   This PR uses the mechanism to test that savemempool fails before the mempool is loaded, and that getmempoolinfo’s “loaded” field is false.

   The main motivation for this change is to enable test coverage for an enhancement of #34020, to have the new IPC methods throw an error while the mempool is loading from disk, rather than an ambiguous empty result.

   The -test=pause_... mechanism may be generally useful for testing other init and shutdown steps.

   The first commit extracts two useful helper methods to the test framework:

   • it adds wait_for_import to the new start_breakable helper right away, though it’s not used until the main commit
   • it changes one node.process.send_signal to self.sigterm_node(node), which I believe is equivalent
   • it merges two different comments around node.process.send_signal for Windows
2. test: move start_breakable and sigterm_node to framework

   - use sigterm_node in break_wait_test, preserving comments
   - extract new start_breakable helper
     - its wait_for_import argument is used in the next commit to
       avoid having to switch self.start_node() to node.start()
       followed by node.wait_for_rpc_connection()

   5d7d4f085c
3. 
   DrahtBot commented at 7:34 pm on March 31, 2026: contributor

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

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #34937 (Fix startup failure with RLIM_INFINITY fd limits by Sjors)
   • #33854 (fix assumevalid is ignored during reindex by Eunovo)

   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.

4. init: add pause_load_mempool test hook

   A new -test=pause_load_mempool argument lets the test framework pause
   node initialization right before loading the mempool. When set, the
   node checks for the existence of a pause_load_mempool file and waits
   until that file is removed.
   
   Use this mechanism to test that savemempool fails before the mempool
   is loaded, and that getmempoolinfo's "loaded" field is false.
   
   Also add a test to ensure the pause can be interrupted.

   b201c7b4ca
5. Sjors force-pushed on Mar 31, 2026
6. DrahtBot added the label CI failed on Mar 31, 2026
7. 
   Sjors commented at 7:41 pm on March 31, 2026: member

   I took the “replace generic comparisons” suggestion from @DrahtBot. There was also a grammar suggestion, but that was for existing text and I want to minimize churn in the first commit. The other suggestion was using a named argument for self.start_node(0 but that seems unnecessary.
8. DrahtBot removed the label CI failed on Mar 31, 2026
9. 
   Sjors commented at 7:24 am on April 1, 2026: member

   Another approach could be to use mock time to control node pauses, e.g. -test= load_mempool_at:1775027951. It would avoid using files, but I don’t think it would be easier to use.

   The -test=pause_... mechanism may be generally useful for testing other init and shutdown steps.

   It would be useful to know if this is actually the case, or if I happened to have found the only place where this mechanism adds coverage. It’s often sufficient to look at the logs after something happened.

10. 
    maflcko commented at 9:39 am on April 1, 2026: member

    It would be useful to know if this is actually the case, or if I happened to have found the only place where this mechanism adds coverage. It’s often sufficient to look at the logs after something happened.

    I think the other places just use try-except, to cover both possible branches (and then hope that the desired branch is covered at leas once). E.g.

     0$ git grep -A19 'encrypted_wallet.walletpassphrase' -- ./test/functional/wallet_importdescriptors.py 
     1test/functional/wallet_importdescriptors.py:        encrypted_wallet.walletpassphrase("passphrase", 99999)
     2test/functional/wallet_importdescriptors.py-        with concurrent.futures.ThreadPoolExecutor(max_workers=1) as thread:
     3test/functional/wallet_importdescriptors.py-            with self.nodes[0].assert_debug_log(expected_msgs=["Rescan started from block 0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206... (slow variant inspecting all blocks)"], timeout=10):
     4test/functional/wallet_importdescriptors.py-                importing = thread.submit(encrypted_wallet.importdescriptors, requests=[descriptor])
     5test/functional/wallet_importdescriptors.py-
     6test/functional/wallet_importdescriptors.py-            # Set the passphrase timeout to 1 to test that the wallet remains unlocked during the rescan
     7test/functional/wallet_importdescriptors.py-            self.nodes[0].cli("-rpcwallet=encrypted_wallet").walletpassphrase("passphrase", 1)
     8test/functional/wallet_importdescriptors.py-
     9test/functional/wallet_importdescriptors.py-            try:
    10test/functional/wallet_importdescriptors.py-                self.nodes[0].cli("-rpcwallet=encrypted_wallet").walletlock()
    11test/functional/wallet_importdescriptors.py-            except JSONRPCException as e:
    12test/functional/wallet_importdescriptors.py-                assert e.error["code"] == -4 and "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before locking the wallet." in e.error["message"]
    13test/functional/wallet_importdescriptors.py-
    14test/functional/wallet_importdescriptors.py-            try:
    15test/functional/wallet_importdescriptors.py-                self.nodes[0].cli("-rpcwallet=encrypted_wallet").walletpassphrasechange("passphrase", "newpassphrase")
    16test/functional/wallet_importdescriptors.py-            except JSONRPCException as e:
    17test/functional/wallet_importdescriptors.py-                assert e.error["code"] == -4 and "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before changing the passphrase." in e.error["message"]
    18test/functional/wallet_importdescriptors.py-
    19test/functional/wallet_importdescriptors.py-            assert_equal(importing.result(), [{"success": True}])
    20test/functional/wallet_importdescriptors.py-
    

    So an alternative could be to fill the mempool dat with enough data to delay it sufficiently.

Contributors
Sjors DrahtBot maflcko