Skip to content

Conversation

@chuksys
Copy link
Contributor

@chuksys chuksys commented Nov 13, 2025

This change uses an alias (LdkChannelDetails) and an explicit Vec<LdkChannelDetails> type annotation for open_channels in close_channel_internal and update_channel_config. This resolves type ambiguity caused by a name collision with the local ChannelDetails struct, which prevents rust-analyzer from correctly inferring the type as Vec, leading to an incorrect len() is private error.

This is a minor fix that would improve the development experience.

This change uses an alias (LdkChannelDetails) and an explicit Vec<LdkChannelDetails> type annotation for 'open_channels' in close_channel_internal and update_channel_config. This resolves type ambiguity caused by a name collision with the local ChannelDetails struct, which prevents rust-analyzer from correctly inferring the type as Vec, leading to an incorrect 'len() is private' error.
@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Nov 13, 2025

I've assigned @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

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

Hmm, can't this be fixed on the rust-analyzer side of things? Not a big fan of making changes just for tooling, especially given that every contributor might run slightly different setups.

@chuksys
Copy link
Contributor Author

chuksys commented Nov 13, 2025

I totally get not wanting to fix things just for tooling.

I think rust-analyzer struggling to infer the correct type likely means we might want to make the code clearer here by using an explicit alias so that it's easy for anyone to know that what is returned here is a Vec of ChannelDetails from LDK and not the local wrapper.

Also I think that since rust-analyzer is a relatively popular tool (over 5 million installs), this small change gives us better readability in addition to the significant developer experience gain!

Let me know if that makes sense - would be happy to hold-off on this if otherwise.

@tnull
Copy link
Collaborator

tnull commented Nov 14, 2025

I think rust-analyzer struggling to infer the correct type likely means we might want to make the code clearer here by using an explicit alias so that it's easy for anyone to know that what is returned here is a Vec of ChannelDetails from LDK and not the local wrapper.

Yeah, I don't buy there is any real confusion here. But anyways, if it improves things for you, there is no real harm here. I just know other people don't like to have such additional clutter in the code. But, until they complain, we can go ahead and merge this for now.

@tnull tnull merged commit 1dd9827 into lightningdevkit:main Nov 14, 2025
17 checks passed
@chuksys
Copy link
Contributor Author

chuksys commented Nov 15, 2025

Thank you!

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.

3 participants