Make Address optional for Opening Channels #490
Conversation
Add a helper method that returns a peer's socket address by first checking active connections and falling back to stored peer information if no active connection exists. This provides a convenient way to look up peer addresses with a preference for currently active connections.
…ddress_if_connected
There was a problem hiding this comment.
Ah, as mentioned offline, I never considered it to be tedious, especially as "usually" people try to open new channels via information parsed from full connection strings, i.e., PUBKEY@ADDR that they copy from somewhere.
I'm not sure we should we should go this way, as I find the proposed API change a bit awkward tbh, since it signals that the address is optional in the general case (opening a/the first channel).
We could consider more fundamental refactoring steps to solve this more cleanly if there is consensus this is a pain, but currently I'm leaning towards it not being worth the effort and/or API breakage. WDYT?
Apart from the approachitself, the changes look already pretty good to me, CI only fails as you missed to adapt one integration test AFAICT.
|
Yeah I see what you're saying on the first time thing. It's a nice to have but also able to be implemented at the user level. I'm good to close this. |
2 participants
Make address optional for open_channel and open_announced_channel if we're already connected to the peer
This takes care of the somewhat frequent case where when we're running a node, we want to open a new channel to a peer we're already connected to, it's a bit tedious to always have to bring the address.
Right now, just pulling the address from peer_manager and peer_store. Could also pull from the graph, but not sure if we'd need to go that far.