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

Websocket not experimental, don't advertize, fix address parsing #6173

Conversation

rustyrussell
Copy link
Contributor

@rustyrussell rustyrussell commented Apr 12, 2023

Changing wireaddr_internal is how I found out it was written into static_channel_backup! So fix that first, then the rest is fairly trivial.

@rustyrussell rustyrussell added this to the v23.08 milestone Apr 12, 2023
@rustyrussell rustyrussell force-pushed the guilt/websocket-not-experimental branch from 334dfed to db35f5a Compare May 26, 2023 07:31
@rustyrussell rustyrussell marked this pull request as ready for review May 26, 2023 07:32
@rustyrussell rustyrussell requested a review from cdecker as a code owner May 26, 2023 07:32
@rustyrussell rustyrussell requested a review from m-schmoock May 26, 2023 07:32
@rustyrussell rustyrussell force-pushed the guilt/websocket-not-experimental branch from 7df636e to 70a6e0c Compare May 29, 2023 05:19
@rustyrussell
Copy link
Contributor Author

Rolled in fixes from @m-schmoock and improved schema, as well as actually updated tests now we've deprecated the old version (which found a bug, fixed!).

@rustyrussell rustyrussell force-pushed the guilt/websocket-not-experimental branch from 70a6e0c to 7d2cc5c Compare May 29, 2023 11:48
@rustyrussell
Copy link
Contributor Author

Trivial rebase onto master to fix clash in generated files....

@rustyrussell rustyrussell force-pushed the guilt/websocket-not-experimental branch 5 times, most recently from 10e907e to 4818d25 Compare May 30, 2023 00:44
This is an internal type: it has no API guarantees (indeed, I'm about
to change it, which is how I discovered scb was using it).

Fortunately for every case we care about, it is actually a wireaddr
(in theory the peer can connect locally using a local socket, but this
is mostly for testing and is a very strange setup, and so simply don't
do scb for those).

In this case, the wire encoding is a single byte followed by the
wireaddr, so open-code that in scb_wire.csv for compatibility.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This contained cut & paste code, and it wasn't clear to me that
the first loop included DNS entries with IPv6 entries.

Instead, allow the iterator to take multiple types, and use
a switch statement so compile will break as new types are added.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. Make it the standard "return the error" pattern.
2. Rather than flags to indicate what types are allowed, have the callers
   check the return explicitly.
3. Document the APIs.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I never really liked this hack: websockets are useful, advertizing
them not so much.

Note that we never actually documented that we would advertize these!

Changelog-EXPERIMENTAL: Protocol: Removed support for advertizing websocket addresses in gossip.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is a major cleanup to how we parse addresses.

1. parse_wireaddr now supports the "dns:" prefix to support dns records (type 5).
2. We are less reliant on separate_address_and_port() which gets confused by
   that colon.
3. We explicitly test every possible address type we can get back from
   parsing, and handle them appropriately.

We update the documentation to use the clearer HOSTNAME vs DNS prefixes now
we also have `dns:` as a prefix.

Changelog-Added: Config: `bind` can now take `dns:` prefix to advertize DNS records.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This obsoletes the use of --announce-addr-dns which I know Michael
didn't really like either.

Changelog-Deprecated: Config: `announce-addr-dns`; use `--bind-addr=dns:ADDR` for finer control.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These are only likely to confuse users, by silently changing behavior.

Changelog-Deprecated: Config: bind-addr=xxx.onion and addr=xxx.onion, use announce=xxx.onion (which was always equivalent).
Changelog-Deprecated: Config: addr=/socketpath, use listen=/socketpath (which was always equivalent).
Now it's not a public type, we need a way to refer to it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Config: `bind=ws:...` to explicitly listen on a websocket.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…d=ws:

Changelog-Deprecated: `experimental-websocket-port`: use `--bind=ws::<portnum>`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell rustyrussell force-pushed the guilt/websocket-not-experimental branch 2 times, most recently from bee38cf to cb5c857 Compare May 30, 2023 05:00
I wasted far too much time on this, disable and reenable later.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell rustyrussell force-pushed the guilt/websocket-not-experimental branch from cb5c857 to ec72619 Compare May 30, 2023 05:16
@rustyrussell rustyrussell merged commit ca34931 into ElementsProject:master May 31, 2023
@evansmj evansmj mentioned this pull request Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants