tests: lnpeer: fix flaky test "hold_invoice_set_doesnt_get_expired"
What the developer wrote
Authored by SomberNight
tests: lnpeer: fix flaky test "hold_invoice_set_doesnt_get_expired"
This test was flaky: the mpp_set resolution gets set to SETTLING several asyncio event loop iterations before the hold invoice callback "cb" gets called.
If the 0.1 sec polling triggers just in the middle of that interval, `assert cb_got_called` fails.
```
async def check_mpp_state():
async def wait_for_resolution():
while True:
await asyncio.sleep(0.1)
if payment_key not in bob_w.received_mpp_htlcs:
continue
if not bob_w.received_mpp_htlcs[payment_key].resolution == RecvMPPResolution.SETTLING:
continue
return
await util.wait_for2(wait_for_resolution(), timeout=2)
> assert cb_got_called
E assert False
tests/test_lnpeer.py:1898: AssertionError
```
see https://github.com/spesmilo/electrum/blob/16c8cb50e38c274cce8f9f66f28d8dd453f9f074/electrum/lnpeer.py#L3136-L3137
fixes https://github.com/spesmilo/electrum/issues/10589
-----
diff to reproduce the failure without present patch:
```
diff --git a/tests/test_lnpeer.py b/tests/test_lnpeer.py
index 8669931c24..e15973d68f 100644
--- a/tests/test_lnpeer.py
+++ b/tests/test_lnpeer.py
@@ -1885,6 +1885,7 @@ class TestPeerDirect(TestPeer):
cb_got_called = False
async def cb(_payment_hash):
self.logger.debug(f"hold invoice callback called. {bob_w.network.get_local_height()=}")
+ await asyncio.sleep(1)
nonlocal cb_got_called
cb_got_called = True
```
What changed, and why it matters
This commit fixes a timing problem in a single automated test for the Lightning Network peer code. It does not change any production code, wallet behavior, or network handling. The fix makes the test wait for a callback event instead of polling every 0.1 seconds, which removes a race condition that could cause the test to fail randomly. There is no security impact for Electrum users.
No security action needed. Treat as a normal test reliability improvement.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch modifies tests/test_lnpeer.py only. It replaces a polling loop that checks whether a received MPP HTLC set has reached RecvMPPResolution.SETTLING with a direct wait on an asyncio.Event that the hold-invoice callback sets. Previously, the test could assert cb_got_called before the callback had actually run, because resolution transitions to SETTLING a few event-loop iterations before the callback executes. The change makes the test deterministic and removes the 0.1 s polling window. No production code is touched.
Changed components
tests/test_lnpeer.pyInspect captured patch +3 / −13
diff --git a/tests/test_lnpeer.py b/tests/test_lnpeer.py
index 8669931..829af60 100644
--- a/tests/test_lnpeer.py
+++ b/tests/test_lnpeer.py
@@ -1882,25 +1882,15 @@ class TestPeerDirect(TestPeer):
del bob_w._preimages[pay_req.rhash] # del preimage so bob doesn't settle
payment_key = bob_w._get_payment_key(lnaddr.paymenthash).hex()
- cb_got_called = False
+ cb_got_called = asyncio.Event()
async def cb(_payment_hash):
self.logger.debug(f"hold invoice callback called. {bob_w.network.get_local_height()=}")
- nonlocal cb_got_called
- cb_got_called = True
+ cb_got_called.set()
bob_w.register_hold_invoice(lnaddr.paymenthash, cb)
async def check_mpp_state():
- async def wait_for_resolution():
- while True:
- await asyncio.sleep(0.1)
- if payment_key not in bob_w.received_mpp_htlcs:
- continue
- if not bob_w.received_mpp_htlcs[payment_key].resolution == RecvMPPResolution.SETTLING:
- continue
- return
- await util.wait_for2(wait_for_resolution(), timeout=2)
- assert cb_got_called
+ await util.wait_for2(cb_got_called.wait(), timeout=2)
mpp_set = bob_w.received_mpp_htlcs[payment_key]
self.assertEqual(mpp_set.resolution, RecvMPPResolution.SETTLING, msg=mpp_set.resolution)
self.assertEqual(len(mpp_set.htlcs), 1, f"should get only one htlc: {mpp_set.htlcs=}")
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.