Make counterparty_node_id optional for close/force-close/update-channel-config

[benthecarman]

When counterparty_node_id is not provided, the server resolves it by looking up the channel via user_channel_id from the channel list.

[ldk-reviews-bot]

👋 Thanks for assigning @tankyleo 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.

[tnull]

Hmm, so far including the counterparty_node_id was a dedicated API design choice in LDK Node to ensure users are positive which exact channel they want to close.

Yes, it's belt-and-suspenders. But, if we think we don't need this, IMO we should change it in LDK Node (which I'm def. open to), rather than having Server's API diverge here?

[benthecarman]

Me and @tankyleo were talking and assumed it was just so we didn't have to iterate over all channels in ldk-node but if it is just for safety i guess that makes sense. It is more annoying for the CLI rather than in code but no real strong feelings here

[ldk-reviews-bot]

🔔 1st Reminder

Hey @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

[tnull]

Me and @tankyleo were talking and assumed it was just so we didn't have to iterate over all channels in ldk-node but if it is just for safety i guess that makes sense. It is more annoying for the CLI rather than in code but no real strong feelings here

Yeah, I'm also not feeling strongly. FWIW, having both precedes the use of user_channel_id here (and for ChannelId you'll need to have counterparty_node_id, too, as they might otherwise not be unique). So, feel free to open a PR on the LDK Node side if you think it's worth removing.

[TheBlueMatt]

Pre-open two channels can have the same channel id so it's definitely required. Obviously the user id should be unique.

[tnull]

Pre-open two channels can have the same channel id so it's definitely required. Obviously the user id should be unique.

Well, note that the API here uses the user_channel_id, not the channel_id. Given we assert the former is unique, we could theoretically drop the counterparty_node_id from the interface.

[TheBlueMatt]

I dunno if LDK Node forces user ids to be random but note that at least in upstream LDK you can reuse user ids as much as you want.

[tnull]

I dunno if LDK Node forces user ids to be random but note that at least in upstream LDK you can reuse user ids as much as you want.

We don't give users the chance to set them freely and force them to be random, exactly in order to be able to use them everywhere as a stable identifier for channels throughout the API.