Skip to content

Expose whether a channel is closing in ChannelDetails #2304

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
valentinewallace opened this issue May 17, 2023 · 4 comments · Fixed by #2347
Closed

Expose whether a channel is closing in ChannelDetails #2304

valentinewallace opened this issue May 17, 2023 · 4 comments · Fixed by #2347
Labels
good first issue Good for newcomers
Milestone

Comments

@valentinewallace
Copy link
Contributor

Currently, there's no definitive way to know that a channel has closed via ChannelDetails. !is_channel_ready && confirmations >= confirmations_required gets you close, but it's still possible that you've just been disconnected to the counterparty since before the channel's confirmation.

We might want to switch from is_channel_ready to an enum with the channel's current status. Or maybe we could add some kind of list_closed_channels util.

@TheBlueMatt TheBlueMatt added the good first issue Good for newcomers label May 18, 2023
@TheBlueMatt TheBlueMatt added this to the 0.0.116 milestone May 18, 2023
@TheBlueMatt
Copy link
Collaborator

Trivial and user-requested so tagging 116.

@henghonglee
Copy link
Contributor

@valentinewallace I'd like to work on this.

Some observations (CMIIW)
1/ is_channel_ready flags (as well as ChannelDetail) are generated only when we want to list_xxx_channels
there is some use in router.rs for channel details to represent routehops but seems from ChannelDetail
documentation that isn't the real intention of the type
2/ ChannelDetail's are a read/write-able persistable thing (not yet sure why since it's generated on-demand).
this might need changes if we switch from is_channel_ready to an enum
3/ Most of the real underly details are dependent on the "true" state in channel.rs using
the ChannelDetails::from_channel fn.

Have a couple of questions.
What is the definition of closed channel right now? and from what I'm reading in the codebase it seems like
self.channel_state == ChannelState::ShutdownComplete as u32 ?
can we base the new enum value off that?

@wpaulino
Copy link
Contributor

1/ is_channel_ready flags (as well as ChannelDetail) are generated only when we want to list_xxx_channels
there is some use in router.rs for channel details to represent routehops but seems from ChannelDetail
documentation that isn't the real intention of the type

Yeah the type is public so you're free to use it as you like, the docs there are more about conveying when a channel will be in such a state.

ChannelDetail's are a read/write-able persistable thing (not yet sure why since it's generated on-demand).
this might need changes if we switch from is_channel_ready to an enum

See the rationale here: #1294. I don't think switching is_channel_ready to an enum applies here, as the docs state, it's just meant to track whether we completed the funding process of the channel. This can be true, and usually will be, for closed channels.

What is the definition of closed channel right now? and from what I'm reading in the codebase it seems like
self.channel_state == ChannelState::ShutdownComplete as u32 ?

This only applies to channels closed cooperatively that have not had their closing transaction confirmed yet. As soon as a closing transaction confirms (regardless of it being cooperative or not), we delete the channel state in ChannelManager.

@henghonglee
Copy link
Contributor

@valentinewallace @wpaulino sorry for the delay, spent some time tracing the code to understand more about channel closures and how they get propagated through the system

currenlty added the shutting down flag as a new field. do let me know if thats not the right direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants