Notify users on channel close

[valentinewallace]

@joemphilips pointed out that the only way to know when a channel closes right now is by polling list_channels. Seems the best way to implement this would be adding a new Event for it.

[joemphilips]

Well, I wanted get notified when a channel is closed because I wanted to take a backup for current state (ChannelMonitor and ChannelManager) whenever a channel is open/closed.
Otherwise I might forget the existence of the channel when my node gets crashed. Thus dangerous. (Or, in case of Channel close, the channel might be zombie )

If I could inject the side effect (in this case disk I/O?) from outside and let RL takes care of the backing up, then there is no need for getting notifying it.
I thought that is the purpose of #681 .

If so, I think we shouldn't add new Event type unless there is any other use case for it.

[TheBlueMatt]

CC #685.

Otherwise I might forget the existence of the channel when my node gets crashed. Thus dangerous.

Note that if you meet the stated requirements for ManyChannelMonitor (namely, the "Note that any updates to a channel's monitor must be applied to each instance of the channel's monitor everywhere (including remote watchtowers) before this function returns." part), you should be just fine, with no need to care about when you go to sync to make sure you don't lose data (though if you never sync, bringing your ChannelManager back up later may force-close channels, as described at https://docs.rs/lightning/0.0.11/lightning/ln/channelmanager/struct.ChannelManagerReadArgs.html#structfield.channel_monitors).

[joemphilips]

Ooooh, I see. Thanks, it took me a while to get my head on straight.
I thought when deserializing ChannelManager, the given number of ChannelMonitors must be exactly the same with the number of the channels in ChannelManager.

I must understand in more detail but at least I'm now sure that I don't have to know exactly when the channel is closed.

I now think having a ChannelClosed event is not bad though, especially reading about #685 and that I don't have to poll list_channels for cleaning up the list of (now-closed) ChannelMonitors on disk.

[TheBlueMatt]

Ooooh, I see. Thanks, it took me a while to get my head on straight.

Probably a great indication that our absent high-level "how do you think about these objects" documentation is a problem :).

I thought when deserializing ChannelManager, the given number of ChannelMonitors must be exactly the same with the number of the channels in ChannelManager.

If that's true, it's a bug. But it is the case that any time a ChannelMonitor is out of sync with the ChannelManager (including not present), we'll force-close the channel with the ChannelMonitor data.

I must understand in more detail but at least I'm now sure that I don't have to know exactly when the channel is closed.

I now think having a ChannelClosed event is not bad though, especially reading about #685 and that I don't have to poll list_channels for cleaning up the list of (now-closed) ChannelMonitors on disk.

You need to wait a little longer for that - the channel closing transactions need to be confirmed first. We desperately need an Event (or call to the ManyChannelMonitor) for that, too, but currently it's incredibly nontrivial to figure out when you can delete ChannelMonitor data.

[TheBlueMatt]

Fixed in #997.