pytest: make hold_timeout.py test plugin release on a prompt, not timeout.
What changed, and why it matters
This commit only changes test code. It replaces a fixed time delay in a test helper plugin with a file-triggered release so tests can control exactly when a fake invoice payment proceeds. There is no change to production code and no security issue.
No security action needed. This is a test-only change improving determinism and reducing CI flakiness.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit modifies tests/plugins/hold_invoice.py and its callers in tests/test_pay.py and tests/test_plugin.py. The plugin previously slept for a configurable ‘holdtime’ seconds inside the invoice_payment hook; now it loops until an ‘unhold’ file appears in the node’s lightning directory. Tests are updated to remove the holdtime option and create the unhold file when they want the HTLC to be released. This is purely a test-infrastructure reliability improvement to reduce flakiness, especially under valgrind.
Changed components
tests/plugins/hold_invoice.pytests/test_pay.pytests/test_plugin.pyInspect captured patch +24 / −11
diff --git a/tests/plugins/hold_invoice.py b/tests/plugins/hold_invoice.py
index b3a0f376..15553cab 100755
--- a/tests/plugins/hold_invoice.py
+++ b/tests/plugins/hold_invoice.py
@@ -3,6 +3,7 @@
"""
from pyln.client import Plugin
+import os
import time
plugin = Plugin()
@@ -10,9 +11,10 @@ plugin = Plugin()
@plugin.hook('invoice_payment')
def on_payment(payment, plugin, **kwargs):
- time.sleep(float(plugin.get_option('holdtime')))
+ # Block until file appears
+ while not os.path.exists("unhold"):
+ time.sleep(0.25)
return {'result': 'continue'}
-plugin.add_option('holdtime', '10', 'The time to hold invoice for.')
plugin.run()
diff --git a/tests/test_pay.py b/tests/test_pay.py
index 6b6875f3..5ab969af 100644
--- a/tests/test_pay.py
+++ b/tests/test_pay.py
@@ -2667,7 +2667,7 @@ def test_setchannel_startup_opts(node_factory, bitcoind):
def test_channel_spendable(node_factory, bitcoind, anchors):
"""Test that spendable_msat is accurate"""
sats = 10**6
- opts = {'plugin': os.path.join(os.getcwd(), 'tests/plugins/hold_invoice.py'), 'holdtime': '30'}
+ opts = {'plugin': os.path.join(os.getcwd(), 'tests/plugins/hold_invoice.py')}
if anchors is False:
opts['dev-force-features'] = "-23"
l1, l2 = node_factory.line_graph(2, fundamount=sats, wait_for_announce=True,
@@ -2693,6 +2693,8 @@ def test_channel_spendable(node_factory, bitcoind, anchors):
# hold_invoice.py plugin.
wait_for(lambda: len(l1.rpc.listpeerchannels()['channels'][0]['htlcs']) == 1)
assert l1.rpc.listpeerchannels()['channels'][0]['spendable_msat'] == Millisatoshi(0)
+ # Tell hold_invoice.py to release hold
+ open(os.path.join(l2.daemon.lightning_dir, TEST_NETWORK, "unhold"), "w").close()
l1.rpc.waitsendpay(payment_hash, TIMEOUT)
# Make sure l2 thinks it's all over.
@@ -2718,6 +2720,7 @@ def test_channel_spendable(node_factory, bitcoind, anchors):
# hold_invoice.py plugin.
wait_for(lambda: len(l2.rpc.listpeerchannels()['channels'][0]['htlcs']) == 1)
assert l2.rpc.listpeerchannels()['channels'][0]['spendable_msat'] == Millisatoshi(0)
+ open(os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, "unhold"), "w").close()
l2.rpc.waitsendpay(payment_hash, TIMEOUT)
@@ -2725,7 +2728,7 @@ def test_channel_receivable(node_factory, bitcoind):
"""Test that receivable_msat is accurate"""
sats = 10**6
l1, l2 = node_factory.line_graph(2, fundamount=sats, wait_for_announce=True,
- opts={'plugin': os.path.join(os.getcwd(), 'tests/plugins/hold_invoice.py'), 'holdtime': '30'})
+ opts={'plugin': os.path.join(os.getcwd(), 'tests/plugins/hold_invoice.py')})
inv = l2.rpc.invoice('any', 'inv', 'for testing')
payment_hash = inv['payment_hash']
@@ -2747,6 +2750,7 @@ def test_channel_receivable(node_factory, bitcoind):
# hold_invoice.py plugin.
wait_for(lambda: len(l2.rpc.listpeerchannels()['channels'][0]['htlcs']) == 1)
assert l2.rpc.listpeerchannels()['channels'][0]['receivable_msat'] == Millisatoshi(0)
+ open(os.path.join(l2.daemon.lightning_dir, TEST_NETWORK, "unhold"), "w").close()
l1.rpc.waitsendpay(payment_hash, TIMEOUT)
# Make sure both think it's all over.
@@ -2773,6 +2777,7 @@ def test_channel_receivable(node_factory, bitcoind):
# hold_invoice.py plugin.
wait_for(lambda: len(l1.rpc.listpeerchannels()['channels'][0]['htlcs']) == 1)
assert l1.rpc.listpeerchannels()['channels'][0]['receivable_msat'] == Millisatoshi(0)
+ open(os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, "unhold"), "w").close()
l2.rpc.waitsendpay(payment_hash, TIMEOUT)
@@ -2786,7 +2791,6 @@ def test_channel_spendable_large(node_factory, bitcoind):
wait_for_announce=True,
opts={
'plugin': os.path.join(os.getcwd(), 'tests/plugins/hold_invoice.py'),
- 'holdtime': '30'
}
)
@@ -2809,6 +2813,7 @@ def test_channel_spendable_large(node_factory, bitcoind):
# Exact amount should succeed.
route = l1.rpc.getroute(l2.info['id'], spendable, riskfactor=1, fuzzpercent=0)['route']
l1.rpc.sendpay(route, payment_hash, payment_secret=inv['payment_secret'])
+ open(os.path.join(l2.daemon.lightning_dir, TEST_NETWORK, "unhold"), "w").close()
l1.rpc.waitsendpay(payment_hash, TIMEOUT)
diff --git a/tests/test_plugin.py b/tests/test_plugin.py
index b48d0c03..12a9d6e5 100644
--- a/tests/test_plugin.py
+++ b/tests/test_plugin.py
@@ -17,7 +17,6 @@ from tests.test_wallet import HsmTool, write_all, WAIT_TIMEOUT
import ast
import copy
import json
-import math
import os
import pytest
import random
@@ -663,14 +662,21 @@ def test_invoice_payment_hook(node_factory):
l2.daemon.wait_for_log('preimage=' + '0' * 64)
-def test_invoice_payment_hook_hold(node_factory):
+def test_invoice_payment_hook_hold(node_factory, executor):
""" l1 uses the hold_invoice plugin to delay invoice payment.
"""
- opts = [{}, {'plugin': os.path.join(os.getcwd(), 'tests/plugins/hold_invoice.py'), 'holdtime': TIMEOUT / 2}]
+ opts = [{}, {'plugin': os.path.join(os.getcwd(), 'tests/plugins/hold_invoice.py')}]
l1, l2 = node_factory.line_graph(2, opts=opts)
inv1 = l2.rpc.invoice(1230, 'label', 'description', preimage='1' * 64)
- l1.rpc.pay(inv1['bolt11'])
+
+ # This should block.
+ f = executor.submit(l1.rpc.pay, inv1['bolt11'])
+ time.sleep(5)
+ assert not f.done()
+
+ open(os.path.join(l2.daemon.lightning_dir, TEST_NETWORK, "unhold"), "w").close()
+ f.result(TIMEOUT)
@pytest.mark.openchannel('v1')
@@ -4034,8 +4040,7 @@ def test_sql(node_factory, bitcoind):
l2.rpc.pay(l3.rpc.invoice(amount_msat=12300, label='inv2', description='description')['bolt11'])
# And I need at least one HTLC in-flight so listpeers.channels.htlcs isn't empty:
- l3.rpc.plugin_start(os.path.join(os.getcwd(), 'tests/plugins/hold_invoice.py'),
- holdtime=int(math.sqrt(TIMEOUT) + 1) * 2)
+ l3.rpc.plugin_start(os.path.join(os.getcwd(), 'tests/plugins/hold_invoice.py'))
inv = l3.rpc.invoice(amount_msat=12300, label='inv3', description='description')
route = l1.rpc.getroute(l3.info['id'], 12300, 1)['route']
l1.rpc.sendpay(route, inv['payment_hash'], payment_secret=inv['payment_secret'])
@@ -4103,6 +4108,7 @@ def test_sql(node_factory, bitcoind):
l3.daemon.wait_for_log("Refreshing channel: {}".format(scid))
# This has to wait for the hold_invoice plugin to let go!
+ open(os.path.join(l3.daemon.lightning_dir, TEST_NETWORK, "unhold"), "w").close()
txid = only_one(l1.rpc.close(l2.info['id'])['txids'])
bitcoind.generate_block(13, wait_for_mempool=txid)
wait_for(lambda: len(l3.rpc.listchannels(source=l1.info['id'])['channels']) == 0)
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.