options: refuse two --announce-addr of the same type.

Gossipd will ignore the second one, but doing it in the front end
gives an explicit error message.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

doc/lightningd-config.5

@@ -2,12 +2,12 @@
 .\"     Title: lightningd-config
 .\"    Author: [see the "AUTHOR" section]
 .\" Generator: DocBook XSL Stylesheets v1.79.1 <http://docbook.sf.net/>
-.\"      Date: 06/20/2018
+.\"      Date: 06/23/2018
 .\"    Manual: \ \&
 .\"    Source: \ \&
 .\"  Language: English
 .\"
-.TH "LIGHTNINGD\-CONFIG" "5" "06/20/2018" "\ \&" "\ \&"
+.TH "LIGHTNINGD\-CONFIG" "5" "06/23/2018" "\ \&" "\ \&"
 .\" -----------------------------------------------------------------
 .\" * Define some portability stuff
 .\" -----------------------------------------------------------------
@@ -279,7 +279,8 @@ An empty \*(AqIPADDRESS\*(Aq is a special value meaning bind to IPv4 and/or
 IPv6 on all interfaces, \*(Aq0\&.0\&.0\&.0\*(Aq means bind to all IPv4
 interfaces, \*(Aq::\*(Aq means \*(Aqbind to all IPv6 interfaces\*(Aq\&.  If \*(AqPORT\*(Aq is
 not specified, 9735 is used\&.  If we can determine a public IP
-address from the resulting binding, the address is announced\&.
+address from the resulting binding, and no other addresses of the
+same type are already announced, the address is announced\&.
 .fi
 .if n \{\
 .RE
@@ -346,7 +347,7 @@ is not specified, a DNS lookup may be done to resolve \*(AqIPADDRESS\*(Aq\&.
 .PP
 \fBannounce\-addr\fR=\fIIPADDRESS[:PORT]|TORADDRESS\&.onion[:PORT]\fR
 .RS 4
-Set an IP address or Tor address to announce; a Tor address is distinguished by ending in
+Set an IP (v4 or v6) address or Tor address to announce; a Tor address is distinguished by ending in
 \fI\&.onion\fR\&.
 \fIPORT\fR
 defaults to 9735\&.
@@ -357,7 +358,7 @@ defaults to 9735\&.
 .nf
 Empty or wildcard IPv4 and IPv6 addresses don\*(Aqt make sense here\&.
 Also, unlike the \*(Aqaddr\*(Aq option, there is no checking that your
-announced addresses are sane (ie\&. not localhost)\&.
+announced addresses are public (e\&.g\&. not localhost)\&.
 .fi
 .if n \{\
 .RE
@@ -368,8 +369,20 @@ announced addresses are sane (ie\&. not localhost)\&.
 .\}
 .nf
 This option can be used multiple times to add more addresses, and
-its use disables autolisten\&.  If necessary, and \*(Aqalways\-use\-proxy\*(Aq
-is not specified, a DNS lookup may be done to resolve \*(AqIPADDRESS\*(Aq\&.
+its use disables autolisten\&.  The spec says you can\*(Aqt announce
+more that one address of the same type (eg\&. two IPv4 or two IPv6
+addresses) so `lightningd` will refuse if you specify more than one\&.
+.fi
+.if n \{\
+.RE
+.\}
+.sp
+.if n \{\
+.RS 4
+.\}
+.nf
+If necessary, and \*(Aqalways\-use\-proxy\*(Aq is not specified, a DNS
+lookup may be done to resolve \*(AqIPADDRESS\*(Aq\&.
 .fi
 .if n \{\
 .RE

doc/lightningd-config.5.txt

@@ -217,7 +217,8 @@ or precisely control where to bind and what to announce with the
     IPv6 on all interfaces, '0.0.0.0' means bind to all IPv4
     interfaces, '::' means 'bind to all IPv6 interfaces'.  If 'PORT' is
     not specified, 9735 is used.  If we can determine a public IP
-    address from the resulting binding, the address is announced.
+    address from the resulting binding, and no other addresses of the
+    same type are already announced, the address is announced.
 
     If the argument begins with 'autotor:' then it is followed by the
     IPv4 or IPv6 address of the Tor control port (default port 9051),
@@ -247,16 +248,20 @@ or precisely control where to bind and what to announce with the
 
 *announce-addr*='IPADDRESS[:PORT]|TORADDRESS.onion[:PORT]'::
 
-    Set an IP address or Tor address to announce; a Tor address is
+    Set an IP (v4 or v6) address or Tor address to announce; a Tor address is
     distinguished by ending in '.onion'.  'PORT' defaults to 9735.
 
     Empty or wildcard IPv4 and IPv6 addresses don't make sense here.
     Also, unlike the 'addr' option, there is no checking that your
-    announced addresses are sane (ie. not localhost).
+    announced addresses are public (e.g. not localhost).
 
     This option can be used multiple times to add more addresses, and
-    its use disables autolisten.  If necessary, and 'always-use-proxy'
-    is not specified, a DNS lookup may be done to resolve 'IPADDRESS'.
+    its use disables autolisten.  The spec says you can't announce
+    more that one address of the same type (eg. two IPv4 or two IPv6
+    addresses) so `lightningd` will refuse if you specify more than one.
+
+    If necessary, and 'always-use-proxy' is not specified, a DNS
+    lookup may be done to resolve 'IPADDRESS'.
 
 *offline*::
     Do not bind to any ports, and do not try to reconnect to any peers.

lightningd/options.c

@@ -181,7 +181,36 @@ static char *opt_add_bind_addr(const char *arg, struct lightningd *ld)
 
 static char *opt_add_announce_addr(const char *arg, struct lightningd *ld)
 {
-	return opt_add_addr_withtype(arg, ld, ADDR_ANNOUNCE, false);
+	const struct wireaddr *wn;
+	size_t n = tal_count(ld->proposed_wireaddr);
+	char *err = opt_add_addr_withtype(arg, ld, ADDR_ANNOUNCE, false);
+	if (err)
+		return err;
+
+	/* Can't announce anything that's not a normal wireaddr. */
+	if (ld->proposed_wireaddr[n].itype != ADDR_INTERNAL_WIREADDR)
+		return tal_fmt(NULL, "address '%s' is not announcable",
+			       arg);
+
+	/* gossipd will refuse to announce the second one, sure, but it's
+	 * better to check and fail now if they've explicitly asked for it. */
+	wn = &ld->proposed_wireaddr[n].u.wireaddr;
+	for (size_t i = 0; i < n; i++) {
+		const struct wireaddr *wi;
+
+		if (ld->proposed_listen_announce[i] != ADDR_ANNOUNCE)
+			continue;
+		assert(ld->proposed_wireaddr[i].itype == ADDR_INTERNAL_WIREADDR);
+		wi = &ld->proposed_wireaddr[i].u.wireaddr;
+
+		if (wn->type != wi->type)
+			continue;
+		return tal_fmt(NULL, "Cannot announce address %s;"
+			       " already have %s which is the same type",
+			       type_to_string(tmpctx, struct wireaddr, wn),
+			       type_to_string(tmpctx, struct wireaddr, wi));
+	}
+	return NULL;
 }
 
 static char *opt_add_ipaddr(const char *arg, struct lightningd *ld)

tests/test_gossip.py

@@ -100,21 +100,22 @@ def test_gossip_disable_channels(node_factory, bitcoind):
     wait_for(lambda: count_active(l2) == 2)
 
 
+@unittest.skipIf(not DEVELOPER, "needs --dev-allow-localhost")
 def test_announce_address(node_factory, bitcoind):
     """Make sure our announcements are well formed."""
 
     # We do not allow announcement of duplicates.
     opts = {'announce-addr':
             ['4acth47i6kxnvkewtm6q7ib2s3ufpo5sqbsnzjpbi7utijcltosqemad.onion',
-             'lldan5gahapx5k7iafb3s4ikijc4ni7gx5iywdflkba5y2ezyg6sjgyd.onion',
              'silkroad6ownowfk.onion',
-             'silkroad7rn2puhj.onion',
              '1.2.3.4:1234',
-             '192.168.1.1',
-             '::',
-             '2001:0db8:85a3:0000:0000:8a2e:0370:7334'],
+             '::'],
             'log-level': 'io'}
     l1, l2 = node_factory.get_nodes(2, opts=[opts, {}])
+
+    # It should warn about the collision between --addr=127.0.0.1:<ephem>
+    # and --announce-addr=1.2.3.4:1234 (may happen before get_nodes returns).
+    wait_for(lambda: l1.daemon.is_in_log('Cannot announce address 127.0.0.1:[0-9]*, already announcing 1.2.3.4:1234'))
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
     scid = l1.fund_channel(l2, 10**6)
     bitcoind.generate_block(5)