lightningd: update graceful notifications when HTLC states change
What changed, and why it matters
This commit fixes a notification bug in Core Lightning's `graceful` JSON-RPC command. The `graceful` command tells users about the next pending payment (HTLC) that is closest to expiring, including its current state. Previously, if a user ran `graceful` while a channel update was in progress, the notification could show a temporary state (like RCVD_ADD_REVOCATION) and never update again when the HTLC reached its final settled state. The fix re-checks and sends updated notifications whenever HTLC states advance during the normal commitment dance. There is no security vulnerability here; it is a correctness and test-flakiness fix for user-facing status notifications.
No security action required. Treat as a normal bug fix / reliability improvement. Reviewers may verify that the added `check_graceful_shutdown` calls do not introduce performance concerns or unintended notification spam; the existing deduplication logic appears to mitigate this.
Security signals we found
No memory safety, authentication, authorization, or cryptographic issues present in diff
Change is purely a notification/state-update correctness fix
No input parsing or external data handling changes
No privilege boundary crossed
Evidence from the diff
The patch adds calls to check_graceful_shutdown(ld) in three HTLC state-advancement handlers in lightningd/peer_htlcs.c: peer_sending_commitsig, peer_got_commitsig, and peer_got_revoke. Previously, check_graceful_shutdown was only invoked on HTLC removal, peer disconnect, and a new graceful invocation. As a result, an outstanding graceful notification could remain stuck on a transient HTLC state observed mid-commitment-dance. The new calls ensure notifications are re-evaluated as the commitment transaction progresses. Identical messages are already deduplicated, so the extra calls are no-ops when no graceful command is outstanding or when the message has not changed. The test test_graceful_htlc is updated to match notifications as an ordered subsequence rather than at exact indices, accommodating the newly emitted transient-state notifications.
Changed components
lightningd/peer_htlcs.ctests/test_misc.pyJSON-RPC graceful command notificationsInspect captured patch +21 / −19
diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c
index acaccff8..52fd398c 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 48dea0a8..ff30642d 100644
--- a/tests/test_misc.py
+++ b/tests/test_misc.py
@@ -4410,20 +4410,17 @@ def test_graceful_htlc(node_factory, executor):
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)
@@ -4434,16 +4431,12 @@ 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: 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]}
Why this scored 17/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.