channel_db: skip non-ASCII DNS hostnames in node announcements

[ekzyis]

BOLT-07 mentions here that senders of node_announcement need to encode DNS hostname bytes with ASCII:

   * `5`: DNS hostname; data = `[1:hostname_len][hostname_len:hostname][2:port]` (length up to 258)
       * `hostname` bytes MUST be ASCII characters.
       * Non-ASCII characters MUST be encoded using Punycode:
         https://en.wikipedia.org/wiki/Punycode

(Punycode maps non-ASCII characters to ASCII characters.)

However, receivers need to check that the bytes are indeed decodable as ASCII. Currently, the code to parse address descriptors would throw UnicodeDecodeError on the first DNS hostname with non-ASCII characters and drop the remaining descriptors.¹ With this PR, such invalid address descriptors are skipped.

FYI, LND additionally checks that the characters are within [a-zA-Z0-9_.] but this is not part of the specification.

Footnotes

1. I haven't tested this against running code, only using the existing tests, so I don't know if Electrum would crash during runtime or not. ↩

[SomberNight]

receivers need to check that the bytes are indeed decodable as ASCII

are we allowed to forward the node announcement message if it fails this check?

[ekzyis]

I have only found this in the spec regarding not forwarding node announcements:

  - if more than one `type 5` address is announced:
    - SHOULD ignore the additional data.
    - MUST not forward the `node_announcement`.

Fwiw, LND does not forward node_announcements if they contain an invalid DNS hostname, see here.

Before, we also did not forward it. With this PR, we do, like the other test cases that fail to parse. Maybe it's better to actually throw like LND?

[accumulator]

To be totally conformant, we should 'punydecode' the hostname, but since at least LND does not seem to do that, it's likely node operators avoid unicode domain names anyway.

[ekzyis]

To be totally conformant, we should 'punydecode' the hostname

I only see the spec mentioning that the sender must encode DNS hostname bytes as ASCII, and Punycode for non-ASCII characters. I don't see where it mentions the receiver needs to decode Punycode back into Unicode. For connection purposes, the Punycode ASCII characters should be enough. Did you mean for display purposes?

since at least LND does not seem to do that, it's likely node operators avoid Unicode domain names anyway.

Yes, I agree, I think Unicode domains are rare.

[accumulator]

I don't see where it mentions the receiver needs to decode Punycode back into Unicode

Indeed, it is actually what DNS resolvers do when resolving a unicode domain name; convert to punycode :)
So no need to decode before presenting to the resolver.