Skip to content
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

chore: gateway list_active_channels command includes funding outpoint #5217

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tvolk131
Copy link
Contributor

@tvolk131 tvolk131 commented May 6, 2024

To close a channel, both LDK and LND require the channel's funding TXID and output index, so we're adding them to the gateway lightning list-active-channels command output to ensure all necessary data is present to close channels

@tvolk131 tvolk131 marked this pull request as ready for review May 6, 2024 23:42
@tvolk131 tvolk131 requested review from a team as code owners May 6, 2024 23:42
Comment on lines +159 to +160
pub channel_point_txid: String,
pub channel_point_output_index: u32,
Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on just making this an OutPoint, that is serialized as <txid>:<out_idx> on the wire?

Comment on lines +790 to +799
let (channel_point_txid, channel_point_output_index) = channel
.channel_point
.split_once(':')
.map(|(txid, output_index)| {
(
txid.to_string(),
output_index.parse::<u32>().unwrap_or_default(),
)
})
.unwrap_or_default();
Copy link
Contributor

Choose a reason for hiding this comment

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

We could actually save that custom parsing logic by just using OutPoit::from_str

@m1sterc001guy
Copy link
Contributor

m1sterc001guy commented May 8, 2024

See #5218 (comment)

We could save needing to do this if we just want to use the remote_pubkey as the key for indicating which channel(s) to close.

Closing channels should (hopefully) be rare, I don't think introducing another RPC call to lookup the necessary info from the remote_pubkey is that bad.

@tvolk131 tvolk131 marked this pull request as draft May 9, 2024 13:04
@tvolk131
Copy link
Contributor Author

tvolk131 commented May 9, 2024

We could save needing to do this if we just want to use the remote_pubkey as the key for indicating which channel(s) to close.

Good point! Converting this to a draft for now, will probably close later.

@justinmoon
Copy link
Contributor

dev call: might not need this anymore ...

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

Successfully merging this pull request may close these issues.

None yet

4 participants