From 9da0aea6874dbc87d1cd53d73cb16932ac7a1194 Mon Sep 17 00:00:00 2001 From: Ken Sedgwick Date: Fri, 10 Jul 2026 15:27:36 -0700 Subject: [PATCH] lightningd: update graceful notifications when HTLC states change The graceful command notifies watchers about the closest-expiry HTLC, including its state, whenever that message changes. But nothing re-evaluated the message when an HTLC changed state: only HTLC removal, peer disconnect and another graceful invocation re-ran the check. If graceful was invoked while a commitment dance was in flight, the initial notification named a transient state (e.g. RCVD_ADD_REVOCATION) and no follow-up ever announced the settled state. test_graceful_htlc waits for exactly that follow-up (since 6994681ae "flake: Fix test_graceful_htlc to be flexible for notifs"), so it times out whenever graceful catches the dance mid-flight, which valgrind CI runs make likely. In one CI failure graceful caught the outgoing HTLC in RCVD_ADD_REVOCATION, the dance completed 600ms later, and no notification followed for the remaining 180 seconds. Re-check graceful progress in the handlers that advance HTLC states (peer_sending_commitsig, peer_got_commitsig, peer_got_revoke). The check is a no-op unless a graceful command is outstanding, and identical messages are already deduplicated. The dance now generates transient-state notifications, so rewrite the test to match expected notifications as an ordered subsequence instead of by exact index. That also removes two accidents the old indexing depended on: the RCVD_ADD_REVOCATION special case (the settled state now always notifies), and the wait for a notification after l1's disconnect, which was really satisfied by the graceful(1) call's own initial notification arriving on the shared rpc socket -- the disconnect itself never notifies, since the message text describes the still-connected peer l3 and does not change. Fixes: https://github.com/ElementsProject/lightning/issues/9219 Changelog-Fixed: JSON-RPC: `graceful` notifications now update when a pending HTLC changes state. --- lightningd/peer_htlcs.c | 9 +++++++++ tests/test_misc.py | 31 ++++++++++++------------------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index acaccff8fa21..52fd398c46f5 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -2275,6 +2275,9 @@ void peer_sending_commitsig(struct channel *channel, const u8 *msg) /* Tell it we've got it, and to go ahead with commitment_signed. */ subd_send_msg(channel->owner, take(towire_channeld_sending_commitsig_reply(msg))); + + /* Maybe graceful wants to know? */ + check_graceful_shutdown(ld); } static bool channel_added_their_htlc(struct channel *channel, @@ -2542,6 +2545,9 @@ void peer_got_commitsig(struct channel *channel, const u8 *msg) /* Tell it we've committed, and to go ahead with revoke. */ msg = towire_channeld_got_commitsig_reply(msg); subd_send_msg(channel->owner, take(msg)); + + /* Maybe graceful wants to know? */ + check_graceful_shutdown(ld); } /* Shuffle them over, forgetting the ancient one. */ @@ -2723,6 +2729,9 @@ void peer_got_revoke(struct channel *channel, const u8 *msg) } wallet_channel_save(ld->wallet, channel); + /* Maybe graceful wants to know? */ + check_graceful_shutdown(ld); + if (penalty_tx == NULL) return; diff --git a/tests/test_misc.py b/tests/test_misc.py index b33704536b4d..296685aaf97e 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -4412,20 +4412,17 @@ def capture(message, **kwargs): fut = executor.submit(run_graceful) - inotif = 0 + # If graceful catches the commitment dance mid-flight, it notifies + # about each transient state (e.g. RCVD_ADD_REVOCATION) on the way, + # so match expected notifications as an ordered subsequence. + seen = [0] - # Wait until graceful has sent at least one HTLC expiry notification - wait_for(lambda: len(notifications) >= inotif + 1) + def wait_notif(expected): + wait_for(lambda: expected in notifications[seen[0]:]) + seen[0] = notifications.index(expected, seen[0]) + 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 + # Once the HTLC is fully committed, we get told. + wait_notif(f'Next HTLC SENT_ADD_ACK_REVOCATION expires at block #118 (10 blocks from now) going to peer {l3.info["id"]} (connected)') # This will tell us about htlcs and the peers (peers unordered) ret = l2.rpc.graceful(1) @@ -4436,16 +4433,12 @@ def capture(message, **kwargs): # Close incoming connection, so incoming HTLC gets stuck. l1.rpc.disconnect(l2.info['id'], force=True) - 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 + # Release the hold so the *outgoing* HTLC resolves: the incoming HTLC + # can't (peer is disconnected), and becomes the next expiry to report. open(os.path.join(l3.daemon.lightning_dir, TEST_NETWORK, "unhold"), "w").close() - 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 + wait_notif(f'Next HTLC SENT_REMOVE_HTLC expires at block #124 (16 blocks from now) coming from peer {l1.info["id"]} (disconnected)') ret = l2.rpc.graceful(1) assert ret == {'pending_htlc_expiries': [124]}