flake: Fix test_graceful_htlc to be flexible for notifs
What the developer wrote
Authored by Dusty Daemon
flake: Fix test_graceful_htlc to be flexible for notifs
The test_graceful_htlc test can receive an extra notification if the test is running slowly enough.
This changes how the test looks at notifications by explicitly incrementing the notification index.
Then, we make it notice and ignore the first RCVD_ADD_REVOCATION notification, which is unwanted in the test but can occur if the server is running slow enough.
Changelog-None
What changed, and why it matters
This commit fixes a flaky automated test, not a security issue. The test sometimes received an extra timing-related notification when the machine was slow. The change makes the test tolerate that extra notification by tracking which notification it is currently checking instead of assuming exactly one notification has arrived. There is no change to production code or to how the software protects users.
No security action needed. Treat as a normal test-stability improvement.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch modifies tests/test_misc.py::test_graceful_htlc only. It replaces fixed-length/last-element assertions on a notifications list with an explicit index (inotif) that advances as the test consumes notifications. It also allows the test to skip an optional RCVD_ADD_REVOCATION notification that can appear depending on timing between sendpay and the graceful RPC call. No C/lightningd source code is changed, and no protocol or cryptographic behavior is altered.
Changed components
tests/test_misc.pyInspect captured patch +19 / −4
diff --git a/tests/test_misc.py b/tests/test_misc.py
index 98432502..91af3d7d 100644
--- a/tests/test_misc.py
+++ b/tests/test_misc.py
@@ -4412,9 +4412,20 @@ def test_graceful_htlc(node_factory, executor):
fut = executor.submit(run_graceful)
+ inotif = 0
+
# Wait until graceful has sent at least one HTLC expiry notification
- wait_for(lambda: len(notifications) == 1)
- wait_for(lambda: notifications[0] == f'Next HTLC SENT_ADD_ACK_REVOCATION expires at block #118 (10 blocks from now) going to peer {l3.info["id"]} (connected)')
+ wait_for(lambda: len(notifications) >= inotif + 1)
+
+ # Depending on on timing between the sendpay and `l2.rpc.graceful`, we may get
+ # RCVD_ADD_REVOCATION or we may be too late to get that.
+ if notifications[inotif] == f'Next HTLC RCVD_ADD_REVOCATION expires at block #118 (10 blocks from now) going to peer {l3.info["id"]} (connected)':
+ # If we get RCVD_ADD_REVOCATION, ignore it and move onto the next notification
+ inotif += 1
+ wait_for(lambda: len(notifications) >= inotif + 1)
+
+ wait_for(lambda: notifications[inotif] == f'Next HTLC SENT_ADD_ACK_REVOCATION expires at block #118 (10 blocks from now) going to peer {l3.info["id"]} (connected)')
+ inotif += 1
# This will tell us about htlcs and the peers (peers unordered)
ret = l2.rpc.graceful(1)
@@ -4425,12 +4436,16 @@ def test_graceful_htlc(node_factory, executor):
# Close incoming connection, so incoming HTLC gets stuck.
l1.rpc.disconnect(l2.info['id'], force=True)
- wait_for(lambda: notifications[-1] == f'Next HTLC SENT_ADD_ACK_REVOCATION expires at block #118 (10 blocks from now) going to peer {l3.info["id"]} (connected)')
+ wait_for(lambda: len(notifications) >= inotif + 1)
+ wait_for(lambda: notifications[inotif] == f'Next HTLC SENT_ADD_ACK_REVOCATION expires at block #118 (10 blocks from now) going to peer {l3.info["id"]} (connected)')
+ inotif += 1
# Release the hold so the *outgoing* HTLC resolves
open(os.path.join(l3.daemon.lightning_dir, TEST_NETWORK, "unhold"), "w").close()
- wait_for(lambda: notifications[-1] == f'Next HTLC SENT_REMOVE_HTLC expires at block #124 (16 blocks from now) coming from peer {l1.info["id"]} (disconnected)')
+ wait_for(lambda: len(notifications) >= inotif + 1)
+ wait_for(lambda: notifications[inotif] == f'Next HTLC SENT_REMOVE_HTLC expires at block #124 (16 blocks from now) coming from peer {l1.info["id"]} (disconnected)')
+ inotif += 1
ret = l2.rpc.graceful(1)
assert ret == {'pending_htlc_expiries': [124]}
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.