pytest: move benchmark in test_connection.py to tests/benchmarks.py
What changed, and why it matters
This commit simply moves an existing performance benchmark test from one test file to another. It does not change any production code, fix any bug, or alter any security behavior. The benchmark checks that disabling a network performance feature called Nagle's algorithm keeps payment messages fast.
No security action needed; this is a test-suite refactoring change.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch relocates test_bench from tests/test_connection.py to tests/benchmark.py, renames it to test_payment_speed, and adapts it to the benchmark framework (using get_bench_node, benchmark(), and tqdm helpers). No C/Python production code, RPC handlers, cryptography, or network socket logic is modified. The test still exercises sendpay/waitsendpay and expects WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS.
Changed components
tests/test_connection.pytests/benchmark.pyInspect captured patch +35 / −36
diff --git a/tests/benchmark.py b/tests/benchmark.py
index ec0062e8..97d26f60 100644
--- a/tests/benchmark.py
+++ b/tests/benchmark.py
@@ -1,7 +1,8 @@
from concurrent import futures
from fixtures import * # noqa: F401,F403
+from pyln.client import RpcError
from tqdm import tqdm
-from utils import (wait_for, TIMEOUT)
+from utils import (wait_for, TIMEOUT, only_one)
import os
@@ -228,3 +229,36 @@ def test_spam_listcommands(node_factory, bitcoind, benchmark):
# This calls "listinvoice" 100,000 times (which doesn't need a transaction commit)
benchmark(l1.rpc.spamlistcommand, 100_000)
+
+
+def test_payment_speed(node_factory, benchmark):
+ """This makes sure we don't screw up nagle handling.
+
+ Normally:
+ Name (time in ms) Min Max Mean StdDev Median IQR Outliers OPS Rounds Iterations
+ test_payment_speed 16.3587 40.4925 27.4874 5.5512 27.7885 8.9291 9;0 36.3803 33 1
+
+ Without TCP_NODELAY:
+ Name (time in ms) Min Max Mean StdDev Median IQR Outliers OPS Rounds Iterations
+ test_payment_speed 153.7132 163.2027 158.6747 3.4059 158.5219 6.3745 3;0 6.3022 9 1
+ """
+ l1 = get_bench_node(node_factory, extra_options={'commit-time': 0})
+ l2 = get_bench_node(node_factory, extra_options={'commit-time': 0})
+
+ node_factory.join_nodes([l1, l2])
+
+ scid = only_one(l1.rpc.listpeerchannels()['channels'])['short_channel_id']
+ routestep = {
+ 'amount_msat': 100,
+ 'id': l2.info['id'],
+ 'delay': 5,
+ 'channel': scid
+ }
+
+ def onepay(l1, routestep):
+ phash = random.randbytes(32).hex()
+ l1.rpc.sendpay([routestep], phash)
+ with pytest.raises(RpcError, match="WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS"):
+ l1.rpc.waitsendpay(phash)
+
+ benchmark(onepay, l1, routestep)
diff --git a/tests/test_connection.py b/tests/test_connection.py
index bd887416..37530022 100644
--- a/tests/test_connection.py
+++ b/tests/test_connection.py
@@ -4579,41 +4579,6 @@ def test_no_delay(node_factory):
assert end < start + 100 * 0.5
-@unittest.skipIf(os.getenv('TEST_BENCH', '0') == '0', "For profiling")
-def test_bench(node_factory):
- """Is our Nagle disabling for critical messages working?"""
- l1, l2 = node_factory.get_nodes(2, opts={'start': False,
- 'commit-time': 0})
-
- # memleak detection plays havoc with profiles.
- del l1.daemon.env["LIGHTNINGD_DEV_MEMLEAK"]
- del l2.daemon.env["LIGHTNINGD_DEV_MEMLEAK"]
-
- l1.start()
- l2.start()
- node_factory.join_nodes([l1, l2])
-
- scid = only_one(l1.rpc.listpeerchannels()['channels'])['short_channel_id']
- routestep = {
- 'amount_msat': 100,
- 'id': l2.info['id'],
- 'delay': 5,
- 'channel': scid
- }
-
- start = time.time()
- # If we were stupid enough to leave Nagle enabled, this would add 200ms
- # seconds delays each way!
- for _ in range(1000):
- phash = random.randbytes(32).hex()
- l1.rpc.sendpay([routestep], phash)
- with pytest.raises(RpcError, match="WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS"):
- l1.rpc.waitsendpay(phash)
- end = time.time()
- duration = end - start
- assert duration == 0
-
-
def test_listpeerchannels_by_scid(node_factory):
l1, l2, l3 = node_factory.line_graph(3, announce_channels=False)
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.