test: speed up functional tests by removing avoidable waits #36440

1. 
   ViniciusCestarii commented at 8:54 PM on October 5, 2026: contributor

   Reduce functional test runtime:

   • set noban_tx_relay = True for tests that can benefit from it.
   • wake P2PInterface.wait_until as soon as a message is delivered or the connection state changes, instead of polling every 50ms.

   Tested on my x86-64 machine:

   Wall time (-j=18): 110.8s to 99.1s (-11%) Accumulated: 1715s to 1411s (-17.5%)

   Tests that were most affected:

   test	before	after
   feature_maxuploadtarget	57.9s	6.2s
   wallet_conflicts	49.2s	7.0s
   p2p_sendheaders	31.1s	6.8s
   p2p_compactblocks	11.1s	1.7s
   wallet_orphanedreward	9.3s	5.2s
   wallet_importprunedfunds	4.9s	2.9s

2. DrahtBot added the label Tests on Oct 5, 2026
3. 
   DrahtBot commented at 8:54 PM on October 5, 2026: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--006a51241073e994b41acfe9ec718e94-->

   External sites

   • Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36440
   • Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36440.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36440#author-ralph

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline and AI policy for information on the review process.

   Type	Reviewers
   ACK	maflcko, w0xlt
   Concept ACK	sedited
   Stale ACK	aaron-leeb, StephenChi-hi, willcl-ark

   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:

   • #36333 _{<img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36333.svg">} (wallet: Delete removeprunedfunds and delete transaction delete code by davidgumberg)
   • #36114 _{<img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36114.svg">} (wallet: harden external signer psbt processing, revamp mock by Sjors)

   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-->

4. ViniciusCestarii force-pushed on Oct 5, 2026
5. DrahtBot added the label CI failed on Oct 5, 2026
6. DrahtBot removed the label CI failed on Oct 5, 2026
7. ViniciusCestarii marked this as ready for review on Oct 5, 2026
8. 
   w0xlt commented at 6:25 PM on October 6, 2026: contributor

   Concept ACK

9. 
   w0xlt commented at 8:06 PM on October 6, 2026: contributor

   ACK 9d7c76d671cdc4e690a176b773d8a9860c37db94

10. 
    aaron-leeb commented at 11:14 PM on October 6, 2026: none

    tACK 9d7c76d

    I ran the functional test suite locally on x86-64 machine w/ 16 cores and noted similar performance improvements:

    Accumulated:

    • Before: 2369 s
    • After: 2081 s ( -12%)

    Runtime:

    • Before: 120 s
    • After: 111 s (-7.5%)
11. 
    StephenChi-hi commented at 3:20 PM on October 7, 2026: none

    tACK 9d7c76d671

    I also tested this locally and confirmed the performance improvements. and some of the most affected tests, for example the wallet_conflicts.py: Before: wallet_conflicts.py | ✓ Passed | 44 s

    After: wallet_conflicts.py | ✓ Passed | 9 s

12. willcl-ark approved
13. 
    willcl-ark commented at 1:35 PM on October 8, 2026: member

    tACK 9d7c76d671cdc4e690a176b773d8a9860c37db94

14. 
    sedited commented at 2:35 PM on October 8, 2026: contributor

    Concept ACK

    I think wallet_change_address.py would be a good addition too (goes from 9s to 1.9s on my machine).

15. sedited requested review from maflcko on Oct 8, 2026
16. test: set noban_tx_relay to speed up slow tx relay tests 10cfeb3335
17. test: wake p2p wait_until on message delivery instead of polling 8dc80cafa2
18. ViniciusCestarii force-pushed on Oct 8, 2026
19. 
    ViniciusCestarii commented at 5:12 PM on October 8, 2026: contributor

    Thanks for the reviews!

    I think wallet_change_address.py would be a good addition too (goes from 9s to 1.9s on my machine).

    True! Looking again, I found some others tests that can benefit from noban_tx_relay = True. There may be even more but checking for it is kinda nuanced and some tests don't benefit much.

    Force-pushed 8dc80cafa2ee5932c9ff499c02d6ab8e3cf2f5ef adding self.noban_tx_relay = True in wallet_exported_watchonly.py, wallet_signer.py and wallet_change_address.py.

    wallet_exported_watchonly.py 7.52s -> 2.68s wallet_signer.py 7.98s -> 2.94s wallet_change_address.py 11.30s -> 2.29s

20. in test/functional/test_framework/util.py:446 in 8dc80cafa2

     440 | @@ -440,6 +441,10 @@ def wait_until_helper_internal(predicate, *, timeout=60, lock=None, timeout_fact
     441 |              with lock:
     442 |                  if predicate():
     443 |                      return
     444 | +                if isinstance(lock, threading.Condition):
     445 | +                    # Wake up early when the lock owner signals a state change
     446 | +                    lock.wait(check_interval)
    

    ---

    ---

    

    maflcko commented at 7:23 PM on October 8, 2026:

    lgtm, but there could be a risk that someone sets a larger explicit interval to ensure something is only checked so often and that for the time in between "nothing" happens.

    I don't think there is such a case in the codebase for a lock, but it would be good to be able to make this reviewable with git grep.

    Can you make all check_interval kwargs? Possibly also timeout:

    -    def wait_until(self, test_function, timeout=60, check_interval=0.05):
    +    def wait_until(self, test_function, *, timeout=60, check_interval=0.05):
    

    (edit: An alternative would be to only run this when the check_interval is the default, but up to you. This is just a nit)

    ---

    

    ViniciusCestarii commented at 1:50 PM on October 9, 2026:

    Thanks! The only caller that can reach the Condition path is P2PInterface.wait_until, which already takes check_interval as keyword-only, so git grep check_interval= already covers it. Making TestNode.wait_until and BitcoinTestFramework.wait_until keyword-only seems like a good consistency change, but it's unrelated to this PR since those never pass a lock.

    Grepping check_interval=, the only tests setting an explicit interval are p2p_ping_ibd.py and interface_http.py, both relying on it for stateful checks. They go through BitcoinTestFramework.wait_until, so they aren't affected.

    Also, my take is that wait_until takes a predicate, which should only describe a state, so evaluating it earlier on a notification should still be correct. Using stateful checks seems like a misuse of wait_until to me. So I'd prefer to leave this as is.

    ---

    

    maflcko commented at 2:21 PM on October 12, 2026:

    Also, my take is that wait_until takes a predicate, which should only describe a state, so evaluating it earlier on a notification should still be correct. Using stateful checks seems like a misuse of wait_until to me.

    It is used twice in:

    test/functional/interface_http.py:            self.wait_until(progress_stalled, check_interval=5)
    test/functional/interface_http.py:        self.wait_until(progress_stalled, check_interval=5)
    

    but maybe they should be re-written to use ensure_for.

21. 
    maflcko commented at 7:24 PM on October 8, 2026: member

    lgtm, but I left a nit

22. 
    maflcko commented at 2:21 PM on October 12, 2026: member

    review ACK 8dc80cafa2ee5932c9ff499c02d6ab8e3cf2f5ef 🏏

    <details><summary>Show signature</summary>

    Signature:

    untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
    RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
    trusted comment: review ACK 8dc80cafa2ee5932c9ff499c02d6ab8e3cf2f5ef 🏏
    yIXD2z6tQ1/5SlEdBHIRIoo1xZ4sNYkuatpfYbydd9viQFehP9L7Reh4+xUoo0ijjQaqyURfe04VenTjN8eLAQ==
    

    </details>

23. DrahtBot requested review from sedited on Oct 12, 2026
24. DrahtBot requested review from w0xlt on Oct 12, 2026
25. DrahtBot requested review from willcl-ark on Oct 12, 2026
26. 
    w0xlt commented at 10:55 PM on October 12, 2026: contributor

    ACK 8dc80cafa2ee5932c9ff499c02d6ab8e3cf2f5ef

27. fanquake merged this on Oct 13, 2026
28. fanquake closed this on Oct 13, 2026

    ---

29. 
    ViniciusCestarii commented at 4:56 AM on October 14, 2026: contributor

    I also plan on reordering the tests in test_runner.py so running in parallel will have a smaller wall time after #36439 is merged too.

Contributors

ViniciusCestariiDrahtBotw0xltaaron-leebStephenChi-hiwillcl-arkseditedmaflcko

Review Requested
willcl-arksedited