Support DNS hostnames in node announcements by remyers · Pull Request #2234 · ACINQ/eclair

remyers · 2022-04-12T15:57:53Z

Add support for DNS host names

Notable changes:

you can now specify a DNS host name as one of your server.public-ips addresses (see PR #911). Note: you can not specify more than one DNS host name.
An IPv4, IPv6 or DNS hostname with port of 0 is invalid as per Bolt #7.
DNS host names will not be resolved until Client attempts to connect to a peer.
will not rebroadcast a NodeAnnouncement that contains more than one DNS host name
the codec does not implement punycode parsing; we should create an issue if we want to support non-ASCII character sets for DNS host names
DNS host name addresses must be resolved to determine if they are IPv4 or IPv6 when deciding whether or not to use a proxy.

To do:

interop testing with c-lightning PR# 4829
~~interop testing with LND Issue# 6337~~
~~interop testing with LDK PR# 1329~~

See also PR #2202 - Use NodeAddress everywhere
and PR #2296 - drop Tor v2 support

remyers · 2022-04-29T14:59:39Z

Simple interop testing with CLN v0.10.2-252-gc0e3155 configured with:
./configure --enable-experimental-features --enable-developer

Overview of setup:

Alice [Eclair] @ dnstest1.co.fr <---> Bob [CLN] @ dnstest2.co.fr <---> Carol [Eclair] @ dnstest3.co.fr

Added to local /etc/hosts:

127.0.0.1       dnstest1.co.fr
127.0.1.1       dnstest2.co.fr
127.0.2.1       dnstest3.co.fr

Created a script dns_gossip.sh to test that DNS hostnames are gossiped between Eclair and CLN.

dns_gossip.sh

Launch bicoind with ./scripts/start-bitcoin.sh, run nodes: alice-eclair & bob-clightning & carol-eclair & and then run the test script ./scripts/dns_gossip.sh.

Example result:

$ ./scripts/dns_gossip.sh 
Alice is 02ca09b2d0fdab441e7e41019e021db360838478484ff191feaf0e49cc13e7a9ba
Bob is 02a821bf4d075dcb88104ab2784ee87ece8979c1e2492c50cac5cdbcaedf686cfc
Carol is 0384f3fca93a670a854f1b9732965b22920c02745f3d91e8084793bf0c7b69f5b5
Adding some Bitcoin to wallets...
Generating a few blocks to confirm wallet balances...
Opening channels between Alice and Bob...
Opening channels between Bob and Carol...
Generating a few blocks to confirm channels...
Creating invoices...
Awaiting gossip sync...
Gossip sync took 14 seconds
Paying invoices...
1. BOB -> CAROL [10,000]
complete
2. ALICE -> BOB [10,000]
payment-sent
3. CAROl -> BOB -> ALICE [5,000]
payment-sent
4. ALICE -> BOB -> CAROL [20,000]
payment-sent
5. BOB -> ALICE [10,000]
complete
6. CAROL -> BOB [5,000]
payment-sent
All invoices paid
"echo All invoices paid" command filed with exit code 0.

To re-run dns_gossip.sh you first must kill the three nodes, reset their databases with ./scripts/reset_all_nodes.sh and then start the nodes again.

remyers · 2022-05-23T11:47:46Z

eclair-core/src/test/scala/fr/acinq/eclair/io/PeerSpec.scala

+    // this actor listens to connection requests and creates connections
+    system.actorOf(ClientSpawner.props(nodeParams.keyPair, nodeParams.socksProxy_opt, nodeParams.peerConnectionConf, TestProbe().ref, TestProbe().ref))
+
+    val invalidDnsHostname_opt = NodeAddress.fromParts("eclair.invalid", 9735).toOption


Note: the .invalid top level domain will never resolve.

…ents

t-bast

The logic looks good to me, I only have small cosmetic/architectural comments, good job!
This does need a rebase though.

docs/release-notes/eclair-vnext.md

eclair-core/src/main/scala/fr/acinq/eclair/router/Announcements.scala

eclair-core/src/main/scala/fr/acinq/eclair/tor/Socks5Connection.scala

eclair-core/src/main/scala/fr/acinq/eclair/router/Validation.scala

eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala

eclair-core/src/test/scala/fr/acinq/eclair/StartupSpec.scala

eclair-core/src/test/scala/fr/acinq/eclair/db/NetworkDbSpec.scala

eclair-core/src/test/scala/fr/acinq/eclair/io/PeerSpec.scala

remyers · 2022-08-01T20:58:45Z

As per Roasbeef at today's meeting, LND 0.15 will propagate DNS hostnames, so nodes running a version from about a month ago should not disappear if they add dns host names to their node announcements.

t-bast

LGTM, a couple of nits and a question for @pm47 about the sun.net dependency.
This needs a rebase, which should be trivial.

eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala

eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/CommonCodecs.scala

eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala

…address

…t" to 127.0.0.1

codecov-commenter · 2022-08-15T15:55:15Z

Codecov Report

Merging #2234 (c2df6d2) into master (33e6fac) will decrease coverage by 0.06%.
The diff coverage is 96.15%.

@@            Coverage Diff             @@
##           master    #2234      +/-   ##
==========================================
- Coverage   84.95%   84.89%   -0.07%     
==========================================
  Files         198      198              
  Lines       15255    15277      +22     
  Branches      633      640       +7     
==========================================
+ Hits        12960    12969       +9     
- Misses       2295     2308      +13

Impacted Files	Coverage Δ
...re/src/main/scala/fr/acinq/eclair/NodeParams.scala	`93.11% <87.50%> (-0.17%)`	⬇️
...ore/src/main/scala/fr/acinq/eclair/io/Client.scala	`56.60% <100.00%> (+0.83%)`	⬆️
...in/scala/fr/acinq/eclair/io/ReconnectionTask.scala	`98.07% <100.00%> (ø)`
...n/scala/fr/acinq/eclair/router/Announcements.scala	`100.00% <100.00%> (ø)`
...main/scala/fr/acinq/eclair/router/Validation.scala	`94.11% <100.00%> (-1.24%)`	⬇️
...n/scala/fr/acinq/eclair/tor/Socks5Connection.scala	`9.37% <100.00%> (+0.71%)`	⬆️
...a/fr/acinq/eclair/wire/protocol/CommonCodecs.scala	`97.05% <100.00%> (+0.04%)`	⬆️
...q/eclair/wire/protocol/LightningMessageTypes.scala	`94.64% <100.00%> (+0.52%)`	⬆️
...q/eclair/channel/publish/ReplaceableTxFunder.scala	`86.91% <0.00%> (-3.67%)`	⬇️
...cala/fr/acinq/eclair/payment/relay/NodeRelay.scala	`95.93% <0.00%> (-1.63%)`	⬇️
... and 5 more

t-bast

LGTM, good job 💯

remyers force-pushed the dnshostnames branch from 2db6978 to 34175b5 Compare April 12, 2022 17:12

t-bast added the Optech Make Me Famous! label Apr 26, 2022

remyers commented May 23, 2022

View reviewed changes

remyers force-pushed the dnshostnames branch from 34175b5 to c1f1e8a Compare May 23, 2022 11:57

Support DNS hostnames and deprecate Torv2 addresses in node announcem…

092d776

…ents

remyers force-pushed the dnshostnames branch from c1f1e8a to 092d776 Compare May 24, 2022 08:06

rorp mentioned this pull request Jun 1, 2022

Drop support for Tor v2 hidden services #2296

Merged

t-bast reviewed Jul 26, 2022

View reviewed changes

remyers changed the title ~~Support DNS hostnames and deprecate Torv2 addresses in announcements~~ Support DNS hostnames in node announcements Jul 26, 2022

remyers added 3 commits July 27, 2022 09:36

Removed Tor v2 deprecation note now in another PR

9d91c5a

Fixed terse brackets of if statements

a9429ba

Move address validateAddresses logic to NodeParams

34e6bef

remyers added 7 commits August 12, 2022 10:22

Added socks5.use-for-dnshostnames param

ec79f2e

Inline dnshostname ascii codec instead of misnamed punycode codec

160db37

Use standard Java functions to detect IPv4/v6 addresses

fd32764

Fixed some code/formatting nits

ff2ac04

List and verify a dnshostname node added to the db

fbf5529

fixup! Use standard Java functions to detect IPv4/v6 addresses

76d24d2

Add assertions to verify that dnshostname is correctly parsed

505c7cb

remyers marked this pull request as ready for review August 12, 2022 10:10

t-bast reviewed Aug 12, 2022

View reviewed changes

remyers and others added 5 commits August 15, 2022 09:21

fixup! Use standard Java functions to detect IPv4/v6 addresses

a8e58ee

Fixed a couple of more formatting nits

e248b7c

Merge branch 'master' into dnshostnames

bb13918

Fix "get proxy address" test failure because of brackets around IPv6 …

ad966d0

…address

Fix test failures caused by fromParts() no longer resolving "localhos…

c2df6d2

…t" to 127.0.0.1

t-bast approved these changes Aug 16, 2022

View reviewed changes

t-bast merged commit bb6148e into ACINQ:master Aug 16, 2022

remyers deleted the dnshostnames branch August 16, 2022 07:38

m-schmoock mentioned this pull request Nov 30, 2022

Re-experimentalize DNS Bolt7 #911 ? ElementsProject/lightning#5756

Closed

Conversation

remyers commented Apr 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

remyers commented Apr 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview of setup:

dns_gossip.sh

Uh oh!

remyers May 23, 2022

Choose a reason for hiding this comment

Uh oh!

t-bast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

remyers commented Aug 1, 2022

Uh oh!

t-bast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Aug 15, 2022

Codecov Report

Uh oh!

t-bast left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

remyers commented Apr 12, 2022 •

edited

Loading

remyers commented Apr 29, 2022 •

edited

Loading