From a430abf89984e1925a276d3d2581ad74c4a2c07d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 30 Mar 2020 19:52:12 +1030 Subject: [PATCH] connectd: permit multiple descriptors of the same type. This restriction was removed from the spec as of 86c2ebcc5973a4133d3ce4d80ae1c203061a1646. We also fix up some strange formatting in that part of the documentation. Changelog-changed: We now announce multiple addresses of the same type, if given. Signed-off-by: Rusty Russell --- connectd/connectd.c | 61 +++++++++++----------------- doc/lightningd-config.5 | 76 +++++++++++++++-------------------- doc/lightningd-config.5.md | 81 ++++++++++++++++++-------------------- tests/test_gossip.py | 13 +++--- 4 files changed, 102 insertions(+), 129 deletions(-) diff --git a/connectd/connectd.c b/connectd/connectd.c index 819661def..cc0da37da 100644 --- a/connectd/connectd.c +++ b/connectd/connectd.c @@ -951,44 +951,21 @@ static void add_binding(struct wireaddr_internal **binding, static int wireaddr_cmp_type(const struct wireaddr *a, const struct wireaddr *b, void *unused) { - /* Returns > 0 if a belongs after b, < 0 if before, == 0 if don't care */ - return (int)a->type - (int)b->type; -} + /* This works, but of course it's inefficient. We don't + * really care, since it's called only once at startup. */ + u8 *a_wire = tal_arr(tmpctx, u8, 0), *b_wire = tal_arr(tmpctx, u8, 0); + int cmp, minlen; -/*~ The spec for we-can't-remember reasons specifies only one address of each - * type. I think there was a bias against "hubs" which would want this. So - * we sort and uniquify. */ -static void finalize_announcable(struct wireaddr **announcable) -{ - size_t n = tal_count(*announcable); + towire_wireaddr(&a_wire, a); + towire_wireaddr(&b_wire, b); - /* BOLT #7: - * - * The origin node: - *... - * - MUST NOT include more than one `address descriptor` of the same - * type. - */ - asort(*announcable, n, wireaddr_cmp_type, NULL); - for (size_t i = 1; i < n; i++) { - /* Note we use > instead of !=: catches asort bugs too. */ - if ((*announcable)[i].type > (*announcable)[i-1].type) - continue; - - status_unusual("WARNING: Cannot announce address %s," - " already announcing %s", - type_to_string(tmpctx, struct wireaddr, - &(*announcable)[i]), - type_to_string(tmpctx, struct wireaddr, - &(*announcable)[i-1])); - - /* Move and shrink; step back because i++ above would skip. */ - memmove(*announcable + i, - *announcable + i + 1, - (n - i - 1) * sizeof((*announcable)[0])); - tal_resize(announcable, --n); - --i; - } + minlen = tal_bytelen(a_wire) < tal_bytelen(b_wire) + ? tal_bytelen(a_wire) : tal_bytelen(b_wire); + cmp = memcmp(a_wire, b_wire, minlen); + /* On a tie, shorter one goes first. */ + if (cmp == 0) + return tal_bytelen(a_wire) - tal_bytelen(b_wire); + return cmp; } /*~ The user can specify three kinds of addresses: ones we bind to but don't @@ -1184,8 +1161,16 @@ static struct wireaddr_internal *setup_listeners(const tal_t *ctx, }; add_announcable(announcable, toraddr); } - /* Sort and uniquify. */ - finalize_announcable(announcable); + + /*~ The spec used to ban more than one address of each type, but + * nobody could remember exactly why, so now that's allowed. */ + /* BOLT #7: + * + * The origin node: + *... + * - MUST place address descriptors in ascending order. + */ + asort(*announcable, tal_count(*announcable), wireaddr_cmp_type, NULL); return binding; } diff --git a/doc/lightningd-config.5 b/doc/lightningd-config.5 index 64cf055f0..e52350534 100644 --- a/doc/lightningd-config.5 +++ b/doc/lightningd-config.5 @@ -362,87 +362,75 @@ precisely control where to bind and what to announce with the Set an IP address (v4 or v6) or automatic Tor address to listen on and (maybe) announce as our node address\. -.nf -.RS + An empty 'IPADDRESS' is a special value meaning bind to IPv4 and/or -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, and no other addresses of the -same type are already announced, the address is announced. +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\. + If the argument begins with 'autotor:' then it is followed by the IPv4 or IPv6 address of the Tor control port (default port 9051), -and this will be used to configure a Tor hidden service for port -9735. The Tor hidden service will be configured to point to the -first IPv4 or IPv6 address we bind to. +and this will be used to configure a Tor hidden service for port 9735\. +The Tor hidden service will be configured to point to the +first IPv4 or IPv6 address we bind to\. + If the argument begins with 'statictor:' then it is followed by the IPv4 or IPv6 address of the Tor control port (default port 9051), -and this will be used to configure a static Tor hidden service for port -9735. The Tor hidden service will be configured to point to the +and this will be used to configure a static Tor hidden service for port 9735\. +The Tor hidden service will be configured to point to the first IPv4 or IPv6 address we bind to and is by default unique to -your nodes id. You can add the text '/torblob=BLOB' followed by up to +your nodes id\. You can add the text '/torblob=BLOB' followed by up to 64 Bytes of text to generate from this text a v3 onion service -address text unique to the first 32 Byte of this text. +address text unique to the first 32 Byte of this text\. You can also use an postfix '/torport=TORPORT' to select the external -tor binding. The result is that over tor your node is accessible by a port +tor binding\. The result is that over tor your node is accessible by a port defined by you and possible different from your local node port assignment + This option can be used multiple times to add more addresses, and -its use disables autolisten. If necessary, and 'always-use-proxy' +its use disables autolisten\. If necessary, and 'always-use-proxy' is not specified, a DNS lookup may be done to resolve 'IPADDRESS' -or 'TORIPADDRESS'. +or 'TORIPADDRESS'\. -.RE - -.fi - \fBbind-addr\fR=\fI[IPADDRESS[:PORT]]|SOCKETPATH\fR Set an IP address or UNIX domain socket to listen to, but do not announce\. A UNIX domain socket is distinguished from an IP address by beginning with a \fI/\fR\. -.nf -.RS + An empty 'IPADDRESS' is a special value meaning bind to IPv4 and/or -IPv6 on all interfaces, '0.0.0.0' means bind to all IPv4 -interfaces, '::' means 'bind to all IPv6 interfaces'. 'PORT' is -not specified, 9735 is used. +IPv6 on all interfaces, '0\.0\.0\.0' means bind to all IPv4 +interfaces, '::' means 'bind to all IPv6 interfaces'\. 'PORT' is +not specified, 9735 is used\. + 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\. If necessary, and 'always-use-proxy' +is not specified, a DNS lookup may be done to resolve 'IPADDRESS'\. -.RE - -.fi - \fBannounce-addr\fR=\fIIPADDRESS[:PORT]|TORADDRESS\.onion[:PORT]\fR 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\. -.nf -.RS -Empty or wildcard IPv4 and IPv6 addresses don't make sense here. + +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 public (e.g. 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. 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. +its use disables autolisten\. + If necessary, and 'always-use-proxy' is not specified, a DNS -lookup may be done to resolve 'IPADDRESS'. +lookup may be done to resolve 'IPADDRESS'\. -.RE - -.fi - \fBoffline\fR Do not bind to any ports, and do not try to reconnect to any peers\. This can be useful for maintenance and forensics, so is usually specified on diff --git a/doc/lightningd-config.5.md b/doc/lightningd-config.5.md index 8e111c362..47de08353 100644 --- a/doc/lightningd-config.5.md +++ b/doc/lightningd-config.5.md @@ -298,65 +298,62 @@ precisely control where to bind and what to announce with the Set an IP address (v4 or v6) or automatic Tor address to listen on and (maybe) announce as our node address. - An empty 'IPADDRESS' is a special value meaning bind to IPv4 and/or - 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, and no other addresses of the - same type are already announced, the address is announced. +An empty 'IPADDRESS' is a special value meaning bind to IPv4 and/or +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. - If the argument begins with 'autotor:' then it is followed by the - IPv4 or IPv6 address of the Tor control port (default port 9051), - and this will be used to configure a Tor hidden service for port - 9735. The Tor hidden service will be configured to point to the - first IPv4 or IPv6 address we bind to. +If the argument begins with 'autotor:' then it is followed by the +IPv4 or IPv6 address of the Tor control port (default port 9051), +and this will be used to configure a Tor hidden service for port 9735. +The Tor hidden service will be configured to point to the +first IPv4 or IPv6 address we bind to. - If the argument begins with 'statictor:' then it is followed by the - IPv4 or IPv6 address of the Tor control port (default port 9051), - and this will be used to configure a static Tor hidden service for port - 9735. The Tor hidden service will be configured to point to the - first IPv4 or IPv6 address we bind to and is by default unique to - your nodes id. You can add the text '/torblob=BLOB' followed by up to - 64 Bytes of text to generate from this text a v3 onion service - address text unique to the first 32 Byte of this text. - You can also use an postfix '/torport=TORPORT' to select the external - tor binding. The result is that over tor your node is accessible by a port - defined by you and possible different from your local node port assignment +If the argument begins with 'statictor:' then it is followed by the +IPv4 or IPv6 address of the Tor control port (default port 9051), +and this will be used to configure a static Tor hidden service for port 9735. +The Tor hidden service will be configured to point to the +first IPv4 or IPv6 address we bind to and is by default unique to +your nodes id. You can add the text '/torblob=BLOB' followed by up to +64 Bytes of text to generate from this text a v3 onion service +address text unique to the first 32 Byte of this text. +You can also use an postfix '/torport=TORPORT' to select the external +tor binding. The result is that over tor your node is accessible by a port +defined by you and possible different from your local node port assignment - 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' - or 'TORIPADDRESS'. +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' +or 'TORIPADDRESS'. **bind-addr**=*\[IPADDRESS\[:PORT\]\]|SOCKETPATH* Set an IP address or UNIX domain socket to listen to, but do not announce. A UNIX domain socket is distinguished from an IP address by beginning with a */*. - An empty 'IPADDRESS' is a special value meaning bind to IPv4 and/or - IPv6 on all interfaces, '0.0.0.0' means bind to all IPv4 - interfaces, '::' means 'bind to all IPv6 interfaces'. 'PORT' is - not specified, 9735 is used. +An empty 'IPADDRESS' is a special value meaning bind to IPv4 and/or +IPv6 on all interfaces, '0.0.0.0' means bind to all IPv4 +interfaces, '::' means 'bind to all IPv6 interfaces'. 'PORT' is +not specified, 9735 is used. - 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'. +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'. **announce-addr**=*IPADDRESS\[:PORT\]|TORADDRESS.onion\[:PORT\]* 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 public (e.g. not localhost). +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 public (e.g. not localhost). - This option can be used multiple times to add more addresses, and - 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. +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'. +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. This diff --git a/tests/test_gossip.py b/tests/test_gossip.py index e678a0755..7c2bb3b72 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -120,9 +120,6 @@ def test_announce_address(node_factory, bitcoind): 'dev-allow-localhost': None} l1, l2 = node_factory.get_nodes(2, opts=[opts, {}]) - # It should warn about the collision between --addr=127.0.0.1: - # 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) @@ -130,8 +127,14 @@ def test_announce_address(node_factory, bitcoind): l1.wait_channel_active(scid) l2.wait_channel_active(scid) - # We should see it send node announce (257 = 0x0101) - l1.daemon.wait_for_log(r"\[OUT\] 0101.*004d010102030404d202000000000000000000000000000000002607039216a8b803f3acd758aa260704e00533f3e8f2aedaa8969b3d0fa03a96e857bbb28064dca5e147e934244b9ba50230032607'") + # We should see it send node announce with all addresses (257 = 0x0101) + # local ephemeral port is masked out. + l1.daemon.wait_for_log(r"\[OUT\] 0101.*54" + "010102030404d2" + "017f000001...." + "02000000000000000000000000000000002607" + "039216a8b803f3acd758aa2607" + "04e00533f3e8f2aedaa8969b3d0fa03a96e857bbb28064dca5e147e934244b9ba50230032607") @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1")