Make address optional in open-channel#159
Make address optional in open-channel#159benthecarman wants to merge 1 commit intolightningdevkit:mainfrom
open-channel#159Conversation
Previously we always required providing the node's address when opening a channel. This was annoying because we may not even need it because we're already connected to the peer. Now we look up the node's address from our peers list if the address is not provided. Also did the same handling as connect peer where you can provide the node and address as pk@addr instead of 2 separate args for easier use in the cli.
|
👋 Hi! This PR is now in draft status. |
| .ok_or_else(|| { | ||
| LdkServerError::new( | ||
| InvalidRequestError, | ||
| "Address is required unless the peer is currently connected. Provide an address or connect-peer first.".to_string(), |
There was a problem hiding this comment.
have you thought about looking up the network graph for the address of the peer ? Maybe that's too far, but pretty sure other node implementations do look at the network graph in case the address is not specified.
There was a problem hiding this comment.
From my testing, lnd doesn't do that. Not sure about CLN or eclair.
Could make sense but imo that should probably be a ldk-node feature where we iterate over all of the announced addresses trying each. We could select the first here but not really a complete feature
There was a problem hiding this comment.
agreed it's better to do this in ldk-node if we do implement this feature.
CLN will look up the gossip map for a socket address to the peer:
If not already connected, fundchannel will automatically attempt to connect if Core Lightning knows a way to contact the node (either from normal gossip, or from a previous connect call).
There was a problem hiding this comment.
Hmm, this was previously proposed here: lightningdevkit/ldk-node#490
I'm still not entirely convinced it's worth making the field optional, as this makes the API for initial channel opens (presumably the common case) more confusing, no? But, I also don't want to block if everybody else agrees it should be done. Though, then we might want to revisit it at the LDK Node level first?
Looking through the rest of the ecosystem, they all (lnd, cln, eclair) don't require you to include the socket address, I feel it'd be better to follow suit as I imagine this is what users expect and its a pretty easy error message to give back before if they are not connected.
Yeah that's probably better: lightningdevkit/ldk-node#832 |
4 participants
Closes #158
Previously we always required providing the node's address when opening a channel. This was annoying because we may not even need it because we're already connected to the peer. Now we look up the node's address from our peers list if the address is not provided.
Also did the same handling as connect peer where you can provide the node and address as pk@addr instead of 2 separate args for easier use in the cli.