lightningd: don't announce names as DNS by default.

This broke BTCPayServer, so revert.  I originally (accidentally!)
implemented this such that it broadcast both DNS and IP entries, but
Michael reported earlier that they still don't propagage well, so
simply suppress them.

Signed-off-by: Rusty Russell <[email protected]>
Fixes: #5795
Changelog-Changeed: Config: `announce-addr-dns` needs to be set to *true* to put DNS names into node announcements, otherwise they are suppressed.
Changelog-Deprecated: Config: `announce-addr-dns` (currently defaults to `false`).  This will default to `true` once enough of the network has upgraded to understand DNS entries.

Author: rustyrussell

doc/lightning-listconfigs.7.md

@@ -101,6 +101,7 @@ On success, an object is returned, containing:
 - **accept-htlc-tlv-types** (string, optional): `accept-extra-tlvs-type` fields from config or cmdline, or not present
 - **tor-service-password** (string, optional): `tor-service-password` field from config or cmdline, if any
 - **dev-allowdustreserve** (boolean, optional): Whether we allow setting dust reserves
+- **announce-addr-dns** (boolean, optional): Whether we put DNS entries into node_announcement
 
 [comment]: # (GENERATE-FROM-SCHEMA-END)
 
@@ -218,4 +219,4 @@ RESOURCES
 ---------
 
 Main web site: <https://github.com/ElementsProject/lightning>
-[comment]: # ( SHA256STAMP:5871ac751654339ed65ab905d61f0bc3afbb8576a33a5c4e9a73d2084f438582)
+[comment]: # ( SHA256STAMP:745268f7f4e4eb19d04ec1a221fbb734d89b4a266049cde3adc3131d86423294)

doc/lightningd-config.5.md

@@ -533,7 +533,7 @@ its use disables autolisten.  If necessary, and 'always-use-proxy'
 is not specified, a DNS lookup may be done to resolve 'DNS' or 'TORIPADDRESS'.
 
   If a 'DNS' hostname was given that resolves to a local interface, the daemon
-will bind to that interface and also announce that as type 'DNS'.
+will bind to that interface: if **announce-addr-dns** is true then it will also announce that as type 'DNS' (rather than announcing the IP address).
 
 * **bind-addr**=*\[IPADDRESS\[:PORT\]\]|SOCKETPATH|DNS\[:PORT\]|DNS\[:PORT\]*
 
@@ -565,9 +565,12 @@ announced addresses are public (e.g. not localhost).
   This option can be used multiple times to add more addresses, and
 its use disables autolisten.
 
-  Since v22.11 'DNS' hostnames can be used for announcement.
-Please note that a lot of mainnet nodes do not yet use, read or propagate this
-information correctly.
+  Since v22.11 'DNS' hostnames can be used for announcement: see **announce-addr-dns**.
+
+* **announce-addr-dns**=*BOOL*
+
+  Set to *true* (default is *false), this so that names given as arguments to **addr** and **announce-addr** are published in node announcement messages as names, rather than IP addresses.  Please note that most mainnet nodes do not yet use, read or propagate this information correctly.
+
 
 * **offline**
 

doc/schemas/listconfigs.schema.json

@@ -294,6 +294,10 @@
     "dev-allowdustreserve": {
       "type": "boolean",
       "description": "Whether we allow setting dust reserves"
+    },
+    "announce-addr-dns": {
+      "type": "boolean",
+      "description": "Whether we put DNS entries into node_announcement"
     }
   }
 }

lightningd/lightningd.c

@@ -206,6 +206,10 @@ static struct lightningd *new_lightningd(const tal_t *ctx)
 	ld->proposed_wireaddr = tal_arr(ld, struct wireaddr_internal, 0);
 	ld->proposed_listen_announce = tal_arr(ld, enum addr_listen_announce, 0);
 
+	/*~ The network is not yet ready for DNS names inside node_announcements,
+	 * so we disable this by default for now. */
+	ld->announce_dns = false;
+
 	ld->remote_addr_v4 = NULL;
 	ld->remote_addr_v6 = NULL;
 	ld->discovered_ip_v4 = NULL;

lightningd/lightningd.h

@@ -234,6 +234,9 @@ struct lightningd {
 	/* If they force db upgrade on or off this is set. */
 	bool *db_upgrade_ok;
 
+	/* Announce names in config as DNS records (recently BOLT 7 addition) */
+	bool announce_dns;
+
 #if DEVELOPER
 	/* If we want to debug a subdaemon/plugin. */
 	const char *dev_debug_subprocess;

lightningd/options.c

@@ -230,6 +230,7 @@ static char *opt_add_addr_withtype(const char *arg,
 	if (is_ipaddr(address)
 	    || is_toraddr(address)
 	    || is_wildcardaddr(address)
+	    || (is_dnsaddr(address) && !ld->announce_dns)
 	    || ala != ADDR_ANNOUNCE) {
 		if (!parse_wireaddr_internal(arg, &wi, ld->portnum,
 					     wildcard_ok, dns_ok, false,
@@ -254,7 +255,7 @@ static char *opt_add_addr_withtype(const char *arg,
 	}
 
 	/* Add ADDR_TYPE_DNS to announce DNS hostnames */
-	if (is_dnsaddr(address) && ala & ADDR_ANNOUNCE) {
+	if (is_dnsaddr(address) && ld->announce_dns && (ala & ADDR_ANNOUNCE)) {
 		/* BOLT-hostnames #7:
 		 * The origin node:
 		 * ...
@@ -1103,6 +1104,10 @@ static void register_opts(struct lightningd *ld)
 	opt_register_early_noarg("--experimental-shutdown-wrong-funding",
 				 opt_set_shutdown_wrong_funding, ld,
 				 "EXPERIMENTAL: allow shutdown with alternate txids");
+	opt_register_early_arg("--announce-addr-dns",
+			       opt_set_bool_arg, opt_show_bool,
+			       &ld->announce_dns,
+			       "Use DNS entries in --announce-addr and --addr (not widely supported!)");
 
 	opt_register_noarg("--help|-h", opt_lightningd_usage, ld,
 				 "Print this message.");

tests/test_gossip.py

@@ -117,7 +117,8 @@ def test_announce_address(node_factory, bitcoind):
     """Make sure our announcements are well formed."""
 
     # We do not allow announcement of duplicates.
-    opts = {'disable-dns': None, 'announce-addr':
+    opts = {'announce-addr-dns': True,
+            'announce-addr':
             ['4acth47i6kxnvkewtm6q7ib2s3ufpo5sqbsnzjpbi7utijcltosqemad.onion',
              '1.2.3.4:1234',
              'example.com:1236',
@@ -158,6 +159,31 @@ def test_announce_address(node_factory, bitcoind):
     assert addresses_dns[0]['port'] == 1236
 
 
+def test_announce_dns_suppressed(node_factory, bitcoind):
+    """By default announce DNS names as IPs"""
+    opts = {'announce-addr': 'example.com:1236',
+            'start': False}
+    l1, l2 = node_factory.get_nodes(2, opts=[opts, {}])
+    # Remove unwanted disable-dns option!
+    del l1.daemon.opts['disable-dns']
+    l1.start()
+
+    # Need a channel so l1 will announce itself.
+    l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
+    scid, _ = l1.fundchannel(l2, 10**6)
+    bitcoind.generate_block(5)
+
+    # Wait for l2 to see l1, with addresses.
+    wait_for(lambda: l2.rpc.listnodes(l1.info['id'])['nodes'] != [])
+    wait_for(lambda: 'addresses' in only_one(l2.rpc.listnodes(l1.info['id'])['nodes']))
+
+    addresses = only_one(l2.rpc.listnodes(l1.info['id'])['nodes'])['addresses']
+    assert len(addresses) == 1
+    assert addresses[0]['type'] == 'ipv4'
+    assert addresses[0]['address'] != 'example.com'
+    assert addresses[0]['port'] == 1236
+
+
 @pytest.mark.developer("gossip without DEVELOPER=1 is slow")
 def test_announce_and_connect_via_dns(node_factory, bitcoind):
     """ Test that DNS annoucements propagate and can be used when connecting.
@@ -176,6 +202,7 @@ def test_announce_and_connect_via_dns(node_factory, bitcoind):
         - 'dev-allow-localhost' must not be set, so it does not resolve localhost anyway.
     """
     opts1 = {'disable-dns': None,
+             'announce-addr-dns': True,
              'announce-addr': ['localhost.localdomain:12345'],  # announce dns
              'bind-addr': ['127.0.0.1:12345', '[::1]:12345']}   # and bind local IPs
     opts3 = {'may_reconnect': True}
@@ -225,7 +252,8 @@ def test_announce_and_connect_via_dns(node_factory, bitcoind):
 def test_only_announce_one_dns(node_factory, bitcoind):
     # and test that we can't announce more than one DNS address
     l1 = node_factory.get_node(expect_fail=True, start=False,
-                               options={'announce-addr': ['localhost.localdomain:12345', 'example.com:12345']})
+                               options={'announce-addr-dns': True,
+                                        'announce-addr': ['localhost.localdomain:12345', 'example.com:12345']})
     l1.daemon.start(wait_for_initialized=False, stderr_redir=True)
     wait_for(lambda: l1.daemon.is_in_stderr("Only one DNS can be announced"))
 
@@ -234,7 +262,7 @@ def test_announce_dns_without_port(node_factory, bitcoind):
     """ Checks that the port of a DNS announcement is set to the corresponding
         network port. In this case regtest 19846
     """
-    opts = {'announce-addr': ['example.com']}
+    opts = {'announce-addr-dns': True, 'announce-addr': ['example.com']}
     l1 = node_factory.get_node(options=opts)
 
     # 'address': [{'type': 'dns', 'address': 'example.com', 'port': 0}]