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

discovery+lnwire: add support for DNS host names in NodeAnnouncement msg #6337

Open
3 tasks
Roasbeef opened this issue Mar 15, 2022 · 8 comments
Open
3 tasks
Labels
beginner Issues suitable for new developers discovery Peer and route discovery / whisper protocol related issues/PRs good first issue Issues suitable for first time contributors to LND p2p Code related to the peer-to-peer behaviour P2 should be fixed if one has time

Comments

@Roasbeef
Copy link
Member

Today it isn't possible to advertise a proper hostname in the NodeAnnouncement message. Historically, we've gotten around this by adding a CLI option that allows users to specify a hostname that should periodically be queried to resolve a new IP to advertise. There exist a spec proposal to add a new address type to the NodeAnnouncement message. With this implemented, we can actually remove our existing hack around this prior lack of protocol feature.

Related to this spec addition is another proposal to add a websockets type. Turns out that most web browsers (read: Chrome) doesn't allow raw websockets (so no TLS layer), which means in practice, one is required to connect via a hostname in either case. As a result, implementing this also gives us a way to expose LN p2p connection over websockets, with the caveat that the node requires a hostname, or uses some existing TPC<->WS proxy.

Steps to Completion

  • Add new host name address type to lnwire.NodeAnnouncement
  • Add a new config flag to allow users to set this new field.
  • Extend the existing peersrpc.UpdateNodeAnnoncement struct to allow dynamic config of this new field
@Roasbeef Roasbeef added p2p Code related to the peer-to-peer behaviour beginner Issues suitable for new developers discovery Peer and route discovery / whisper protocol related issues/PRs labels Mar 15, 2022
@Roasbeef Roasbeef added this to the v0.16.0 milestone Mar 15, 2022
@gabbyprecious
Copy link
Contributor

Hi @Roasbeef can I be assigned to this?

@DeGrandis
Copy link

DeGrandis commented Apr 7, 2022

Hi @Roasbeef , I would like to give this a go too. I'm new to this repo and would like to help. Maybe pairing with @gabbyprecious ?

Edit: after poking around, this looks to be over my head, for now. I will look for something else.

@guggero
Copy link
Collaborator

guggero commented Apr 11, 2022

We don't normally assign issues to external contributors. But if you want to "claim" something, you can always create a draft (!) pull request and link the issue in the description. That way anyone looking at the issue sees that someone is working on it.

@tvolk131
Copy link
Contributor

There exist a spec proposal to add a new address type to the NodeAnnouncement message.

@Roasbeef can you link the spec you're talking about? All I could find was this thread, but this doesn't seem like quite what you're proposing.

@remyers
Copy link

remyers commented Jul 5, 2022

Is this the PR to add support for the spec change: BOLT 7: add gossip address descriptor type DNS hostname #911 ?

@Roasbeef
Copy link
Member Author

@tvolk131 yep see the linked PR above ^

@m-schmoock
Copy link

m-schmoock commented Jul 28, 2022

Hi @Roasbeef
lightning/bolts#911 finally reached interop with Rust-Lightning and is going being merged soon. I was thinking about removing the EXPERIMENTAL checks in CLN code for the next release. But there was chatter in our Discord that LND currently won't propagate node_annoucement's that contain (optional) DNS hostnames as address descriptors. About that:

  • Is this correct?
  • If yes, is there an issue on your GitHub for this? (I couldn't find it)
  • We likely can't remove CLN EXPERIMENTAL when LND, which is the biggest part of the network, won't propagate the node_annoucement's

@Roasbeef
Copy link
Member Author

But there was chatter in our Discord that LND currently won't propagate node_annoucement's that contain (optional) DNS hostnames as address descriptors. About that:

No this is fixed now, we'll propagate new addrs that we don't yet understand.

@saubyk saubyk modified the milestones: v0.16.0, v0.16.1 Dec 6, 2022
@saubyk saubyk added the P2 should be fixed if one has time label Aug 8, 2023
@saubyk saubyk removed this from the Medium Priority milestone Aug 8, 2023
@ziggie1984 ziggie1984 added the good first issue Issues suitable for first time contributors to LND label Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginner Issues suitable for new developers discovery Peer and route discovery / whisper protocol related issues/PRs good first issue Issues suitable for first time contributors to LND p2p Code related to the peer-to-peer behaviour P2 should be fixed if one has time
Projects
None yet
Development

No branches or pull requests

9 participants