websocket address type: allow transport over RFC6455 #891
Conversation
And allow transparent upgrade. This allows a wider range of software to interact with the lightning network. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
|
Is this better off done via a bridge instead? Last I heard there weren’t any decent websocket implementations that weren’t huge, which (maybe incorrectly) led me to understand that it was reasonably complicated. In either case, it seems if the only options are fresh code or huge dependency, it’s probably a bad idea for us to recommend users integrate it instead of simply using one of the websocket bridge services.
… On Jul 27, 2021, at 01:31, Rusty Russell ***@***.***> wrote:
And allow transparent upgrade. This allows a wider range of software to interact with the lightning network.
You can view, comment on, or merge this pull request online at:
#891
Commit Summary
option_websocket: allow transport over RFC6455
File Changes
M .aspell.en.pws (2)
M 08-transport.md (47)
M 09-features.md (2)
Patch Links:
https://github.com/lightningnetwork/lightning-rfc/pull/891.patch
https://github.com/lightningnetwork/lightning-rfc/pull/891.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
@TheBlueMatt has a valid point here, we used https://github.com/novnc/websockify at my old workplace. |
Go has its own implementation that's packaged as part of the standard library, so we effectively get this for free. The advantage of simply letting a node advertise that it supports doing the brontide handshake over WS is that you can avoid needing to use a proxy all together. |
08-transport.md
Outdated
| The responder: | ||
| - if it supports `option_websocket`: | ||
| - SHOULD set `option_websocket` in its node announcements | ||
| - MUST attempt WebSocket upgrade if the Act 1 handshake it receives |
There was a problem hiding this comment.
See comment above re separation of the layers. When nodes connect over Tor today, brontide implementations don't need to do anything different, and we can inherit that behavior here by defining the web sockets feature as a new address type.
There was a problem hiding this comment.
Also by having it as a new address type, nodes can advertise another addr here, which might actually just be a bridge they're using.
|
I feel like this would be a good candidate for a bLIP - I don't think we will reasonably consider this something that "most nodes should implement", but instead a totally-optional cool thing for those for whom its easy. |
|
I was originally going to add a new address type, but a single bit is cheaper. I'll see what I can do there, though.... |
Everyone hated the overload. But it's not a full address type, so make it just a port number as a compromise. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell
changed the title
Yes, I was looking for implementations and had the same sinking feeling. But it's actually just modern software which sucks ass. This is a minimal proxy implementation which I made from reading the RFC; it's 350 well-commented lines: |
07-routing-gossip.md
Outdated
| @@ -285,6 +285,7 @@ The following `address descriptor` types are defined: | |||
| onion service addresses; Encodes: | |||
| `[32:32_byte_ed25519_pubkey] || [2:checksum] || [1:version]`, where | |||
| `checksum = sha3(".onion checksum" | pubkey || version)[:2]`. | |||
| * `5`: WebSocket port; data = `[2:port]` (length 2) | |||
There was a problem hiding this comment.
Doesn't this also need an interface/IP? Or it's meant to be a sort of modifier?
There was a problem hiding this comment.
The alternative is to duplicate all the entries, so this is simply a modifier.
07-routing-gossip.md
Outdated
| to 0. | ||
| - SHOULD ensure `ipv4_addr` AND `ipv6_addr` are routable addresses. | ||
| - MUST set `features` according to [BOLT #9](09-features.md#assigned-features-flags) | ||
| - SHOULD set `flen` to the minimum length required to hold the `features` | ||
| bits it sets. | ||
| - MUST NOT add a `type 5` address unless there is also at least one address of different type. |
There was a problem hiding this comment.
What's the rationale here? To ensure a node is able to serve the "greater" network w/ a normal TCP (or w/e) transport?
There was a problem hiding this comment.
Yeah, there's already a js library which talks WS to this: https://github.com/rustyrussell/bolt12/
It's kinda cute...
rustyrussell
dismissed
Roasbeef’s stale review
Done, it's now a new addr type (i.e. an alternate port for existing addrs)
|
We have a clash here with #911 DNS descriptors. |
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
|
@rustyrussell @TheBlueMatt |
| @@ -285,6 +285,7 @@ The following `address descriptor` types are defined: | |||
| onion service addresses; Encodes: | |||
| `[32:32_byte_ed25519_pubkey] || [2:checksum] || [1:version]`, where | |||
| `checksum = sha3(".onion checksum" | pubkey || version)[:2]`. | |||
| * `6`: WebSocket port; data = `[2:port]` (length 2) | |||
There was a problem hiding this comment.
I'd rather we allow specification of an IP as well here. Though maybe that interacts poorly with the hostname gossip thing? There may likely end up being nodes that would love to allow websocket connections, but sit behind cloudflare/nginx/whatever when doing so, whereas native LN protocol obviously doesn't proxy through a web proxy. Thus, they'd want to be able to specify a different host for websockets. I suppose for cloudflare-proxied nodes (dear god people stop using cloudflare) you'd need a domain name, not an IP, and have to send the HTTP Host header.
Maybe this should be hostname-only? Or maybe two different gossips, one for hostnames and one for ips?
There was a problem hiding this comment.
You basically iterate through all the other addresses, trying this port. It's not ideal, but it seemed easier than some kind of specification which indicated which other descriptors it applied to...
There was a problem hiding this comment.
Yea, totally fair, I think my broader point here is that we really should consider wss a required part of this, because supporting websockets so browser clients can connect but then not support wss so browser clients can't connect in a secure context seems like it almost completely neuters the value here, sadly. For wss you need a hostname, ultimately, and I think we'd probably want to support a different hostname than the lightning node itself, as the wss part will probably be nginx or some other proxy, hence...I think we should just shove a hostname here :(
There was a problem hiding this comment.
+1 for wanting wss, we've created a websocket to tcp proxy server for connecting in a browser, but we can't do ws unfortunately.
And allow transparent upgrade.Allows you to advertize an alternate port. This allows a wider range of software to interact with the lightning network.