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

pull ViniciusCestarii wants to merge 2 commits into bitcoin:master from ViniciusCestarii:speedup-p2p-tests changing 8 files +25 −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

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


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



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


Reviews

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

Type Reviewers

Concept ACK sedited

Stale ACK w0xlt, aaron-leeb, StephenChi-hi, willcl-ark

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #36306 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36306.svg"></sub> (test: opt flaky functional tests into immediate tx relay by ronnakamoto)
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.
ViniciusCestarii force-pushed on Oct 5, 2026
DrahtBot added the label CI failed on Oct 5, 2026
DrahtBot removed the label CI failed on Oct 5, 2026
ViniciusCestarii marked this as ready for review on Oct 5, 2026
w0xlt commented at 6:25 PM on October 6, 2026: contributor

Concept ACK
w0xlt commented at 8:06 PM on October 6, 2026: contributor

ACK 9d7c76d671cdc4e690a176b773d8a9860c37db94
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%)
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
willcl-ark approved
willcl-ark commented at 1:35 PM on October 8, 2026: member

tACK 9d7c76d671cdc4e690a176b773d8a9860c37db94
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).
sedited requested review from maflcko on Oct 8, 2026
test: set noban_tx_relay to speed up slow tx relay tests 10cfeb3335
test: wake p2p wait_until on message delivery instead of polling 8dc80cafa2
ViniciusCestarii force-pushed on Oct 8, 2026
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
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)
maflcko commented at 7:24 PM on October 8, 2026: member

lgtm, but I left a nit