Skip to content

Plugins: 'channel_closed' notification type#8048

Closed
NishantBansal2003 wants to merge 3 commits intoElementsProject:masterfrom
NishantBansal2003:closechannel_notification
Closed

Plugins: 'channel_closed' notification type#8048
NishantBansal2003 wants to merge 3 commits intoElementsProject:masterfrom
NishantBansal2003:closechannel_notification

Conversation

@NishantBansal2003
Copy link
Contributor

@NishantBansal2003 NishantBansal2003 commented Feb 3, 2025 •
edited
Loading

Ref: #3207 #3662

PR Description

This adds a new notification type: channel_closed.

This notification is emitted if a channel has been successfully closed with a peer and sends the peer's ID and the closing transaction ID.

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

  • The changelog has been updated in the relevant commit(s) according to the guidelines.
  • Tests have been added or modified to reflect the changes.
  • Documentation has been reviewed and updated as needed.
  • Related issues have been listed and linked, including any that this PR closes.

@NishantBansal2003
Copy link
Contributor Author

NishantBansal2003 commented Feb 3, 2025 •
edited
Loading

Although the channel closure state can currently be notified via channel_state_changed, it would be more symmetrical to have a dedicated channel_closed event analogous to the channel_opened event.
cc: @rustyrussell

@NishantBansal2003 NishantBansal2003 force-pushed the closechannel_notification branch from 10c1da0 to c604e89 Compare February 15, 2025 03:36
@chrisguida
Copy link
Contributor

chrisguida commented Feb 27, 2025

Curious if you could elaborate on what problems you were running into with channel_state_changed

@NishantBansal2003
Copy link
Contributor Author

NishantBansal2003 commented Feb 28, 2025

Curious if you could elaborate on what problems you were running into with channel_state_changed

There were no issues. However, this new channel_closed notification type serves two purposes:

  1. To maintain symmetry by providing a channel_closed event analogous to the channel_opened event (as mentioned in Plugin notification for when UTXOs are spent? #3207).
  2. To address feedback from a plugin developer on IRC, who noted that a channel_closed notification would be a welcome addition (as mentioned in Add new notifications for plugins #3662).

Lagrang3
Lagrang3 reviewed Mar 11, 2025
View reviewed changes
Copy link
Collaborator

@Lagrang3 Lagrang3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is looking good to me.

Now, if I was going to use this notification instead of channel_state_changed
I would probably need more information on the channel that failed.
For instance, just like channel_state_changed I would like to have:

  • the peer_id, that you called id,
  • the channel_id,
  • the short_channel_id,
  • and the cause.

@NishantBansal2003
lightningd/notification: add 'channel_closed' notification
4e80747 
Changelog-Added: a new 'channel_closed' notification type
is added, which is emitted when a channel has
been successfully closed with a peer.
Signed-off-by: Nishant Bansal <[email protected]>
@NishantBansal2003
pytest: Add a test for the 'channel_closed' subscription and notifica…
2fa207f 
…tion

Changelog-None
Signed-off-by: Nishant Bansal <[email protected]>
@NishantBansal2003
doc: document 'channel_closed' notification.
56e045e 
Changelog-None
Signed-off-by: Nishant Bansal <[email protected]>
@NishantBansal2003 NishantBansal2003 force-pushed the closechannel_notification branch from c604e89 to 56e045e Compare March 12, 2025 12:25
@whitslack whitslack mentioned this pull request Aug 16, 2025
Merged
4 tasks
@rustyrussell
Copy link
Contributor

rustyrussell commented Oct 2, 2025

With the recent addition of the CLOSED state notification for channel_state_changed, I think that is now a superset of this, and probably more useful. So I'm tempted to close this one, if there are no objections?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Lagrang3 Lagrang3 Awaiting requested review from Lagrang3

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@NishantBansal2003 @chrisguida @rustyrussell @Lagrang3