Websocket fix

[rustyrussell]

I noticed that experimental-websocket-port didn't work on my node (it crashed). Along the way, I found a bug in DNS handling.

[SimonVrouwe]

Maybe off-topic, when I run test_announce_and_connect_via_dns on my local machine it fails with:

connectd: DNS with getaddrinfo gave: Name or service not known

Because that test expects a localhost.localdomain to be present?
'announce-addr': ['localhost.localdomain:12345'], # announce dns

But my machine (Debian) has .lan instead of .localdomain:

$ host localhost
localhost.lan has address 127.0.0.1
localhost.lan has IPv6 address ::1

Changing .localdomain to .lan makes the test pass, but is it correct?

[m-schmoock]

Changing .localdomain to .lan makes the test pass, but is it correct?

Yes it works, no its not correct. I was the assumption that "localdomain" is some sort of standard. Turns out it isn't. I didn't want to go for just "localhost" (which would work) because it can trigger certain internal code paths that shortcut address resolution to 127.0.0.1. I wanted the test to be in a way that it needs to run for getaddrinfo and let the system decide.

[m-schmoock]

Sure, when Websockets have empty addresses this should now work. Good that its now test covered.

Imho, the way opt_add_addr_withtype works is a little too iffy. But I also don't have a better idea.

[SimonVrouwe]

Changing .localdomain to .lan makes the test pass, but is it correct?

Yes it works, no its not correct. I was the assumption that "localdomain" is some sort of standard. Turns out it isn't. I didn't want to go for just "localhost" (which would work) because it can trigger certain internal code paths that shortcut address resolution to 127.0.0.1. I wanted the test to be in a way that it needs to run for getaddrinfo and let the system decide.

It looks you can test what you have locally (.lan or .localdomain) using:
socket.getaddrinfo('localhost.localdomain', '12345')
returning error (or not): gaierror: [Errno -2] Name or service not known

[m-schmoock]

One stupid fix would be to add 127.0.0.1 localhost.localdomain to your /etc/hosts. But thats just a temporary solution if your annoyed by the test failing.

[SimonVrouwe]

One stupid fix would be to add 127.0.0.1 localhost.localdomain to your /etc/hosts. But thats just a temporary solution if your annoyed by the test failing.

Thanks, add it as an alias in /etc/hosts, like this:
127.0.0.1 localhost localhost.localdomain
and still works after restarting NetworkManager, so seems permanent.

[rustyrussell]

We should reduce the scope of the localhost parsing perhaps: use it as a fallback iff no_dns is set, otherwise resolve it like anything else?

[vincenzopalazzo]

ack 729eace

CI looks like fixed with #4954

[m-schmoock]

ack 729eace

[rustyrussell]

Trivial rebase on master to fix flake.

[m-schmoock]

ACK 6c21a6b

I restarted CI because it failed at flaky but unrelated tests/test_pay.py::test_fetchinvoice_3hop. Now its all good.

[m-schmoock]

Whohoo, didn't know I had the permissions to do so, but the merge worked <3