Skip to content

Commit

Permalink
lightningd: really do allow two Torv3 addresses.
Browse files Browse the repository at this point in the history 
This surprised me, since the CHANGELOG for [0.8.2] said:

	We now announce multiple addresses of the same type, if given. ([3609](#3609))

But it lied!

Changelog-Fixed: We really do allow providing multiple addresses of the same type.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
  • Loading branch information
@rustyrussell @cdecker
rustyrussell authored and cdecker committed Nov 14, 2021
1 parent 3dcab97 commit 9d18180
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 67 deletions.
74 changes: 26 additions & 48 deletions common/test/run-wireaddr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,39 +112,6 @@ void towire_u8_array(u8 **pptr UNNEEDED, const u8 *arr UNNEEDED, size_t num UNNE
{ fprintf(stderr, "towire_u8_array called!\n"); abort(); }
/* AUTOGENERATED MOCKS END */

static bool wireaddr_internal_eq(const struct wireaddr_internal *a,
const struct wireaddr_internal *b,
bool cmp_torservice_blob)
{
if (a->itype != b->itype)
return false;

switch (a->itype) {
case ADDR_INTERNAL_SOCKNAME:
return streq(a->u.sockname, b->u.sockname);
case ADDR_INTERNAL_ALLPROTO:
return a->u.port == b->u.port;
case ADDR_INTERNAL_AUTOTOR:
case ADDR_INTERNAL_STATICTOR:
if (!wireaddr_eq(&a->u.torservice.address,
&b->u.torservice.address))
return false;
if (a->u.torservice.port != b->u.torservice.port)
return false;
if (!cmp_torservice_blob)
return true;
return memeq(a->u.torservice.blob, sizeof(a->u.torservice.blob),
b->u.torservice.blob, sizeof(b->u.torservice.blob));
case ADDR_INTERNAL_FORPROXY:
if (!streq(a->u.unresolved.name, b->u.unresolved.name))
return false;
return a->u.unresolved.port == b->u.unresolved.port;
case ADDR_INTERNAL_WIREADDR:
return wireaddr_eq(&a->u.wireaddr, &b->u.wireaddr);
}
abort();
}

int main(int argc, char *argv[])
{
const char *err;
Expand All @@ -155,87 +122,98 @@ int main(int argc, char *argv[])
assert(parse_wireaddr_internal("127.0.0.1", &addr, DEFAULT_PORT, false, false, false, false, &err));
expect->itype = ADDR_INTERNAL_WIREADDR;
assert(parse_wireaddr("127.0.0.1:9735", &expect->u.wireaddr, 0, NULL, &err));
assert(wireaddr_internal_eq(&addr, expect, false));
assert(wireaddr_internal_eq(&addr, expect));

/* IPv4 address with port. */
assert(parse_wireaddr_internal("127.0.0.1:1", &addr, DEFAULT_PORT, false, false, false, false, &err));
expect->itype = ADDR_INTERNAL_WIREADDR;
assert(parse_wireaddr("127.0.0.1:1", &expect->u.wireaddr, 0, NULL, &err));
assert(wireaddr_internal_eq(&addr, expect, false));
assert(wireaddr_internal_eq(&addr, expect));

/* Simple IPv6 address. */
assert(parse_wireaddr_internal("::1", &addr, DEFAULT_PORT, false, false, false, false, &err));
expect->itype = ADDR_INTERNAL_WIREADDR;
assert(parse_wireaddr("::1", &expect->u.wireaddr, DEFAULT_PORT, NULL, &err));
assert(wireaddr_internal_eq(&addr, expect, false));
assert(wireaddr_internal_eq(&addr, expect));

/* IPv6 address with port. */
assert(parse_wireaddr_internal("[::1]:1", &addr, DEFAULT_PORT, false, false, false, false, &err));
expect->itype = ADDR_INTERNAL_WIREADDR;
assert(parse_wireaddr("::1", &expect->u.wireaddr, 1, NULL, &err));
assert(wireaddr_internal_eq(&addr, expect, false));
assert(wireaddr_internal_eq(&addr, expect));

/* autotor address */
assert(parse_wireaddr_internal("autotor:127.0.0.1", &addr, DEFAULT_PORT, false, false, false, false, &err));
expect->itype = ADDR_INTERNAL_AUTOTOR;
expect->u.torservice.port = DEFAULT_PORT;
assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9051, NULL, &err));
assert(wireaddr_internal_eq(&addr, expect, false));
assert(wireaddr_internal_eq(&addr, expect));

/* autotor address with port */
assert(parse_wireaddr_internal("autotor:127.0.0.1:9055", &addr, DEFAULT_PORT, false, false, false, false, &err));
expect->itype = ADDR_INTERNAL_AUTOTOR;
expect->u.torservice.port = DEFAULT_PORT;
assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9055, NULL, &err));
assert(wireaddr_internal_eq(&addr, expect, false));
assert(wireaddr_internal_eq(&addr, expect));

/* autotor address with torport */
assert(parse_wireaddr_internal("autotor:127.0.0.1/torport=9055", &addr, DEFAULT_PORT, false, false, false, false, &err));
expect->itype = ADDR_INTERNAL_AUTOTOR;
expect->u.torservice.port = 9055;
assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9051, NULL, &err));
assert(wireaddr_internal_eq(&addr, expect, false));
assert(wireaddr_internal_eq(&addr, expect));

/* autotor address with port and torport */
assert(parse_wireaddr_internal("autotor:127.0.0.1:9055/torport=10055", &addr, DEFAULT_PORT, false, false, false, false, &err));
expect->itype = ADDR_INTERNAL_AUTOTOR;
expect->u.torservice.port = 10055;
assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9055, NULL, &err));
assert(wireaddr_internal_eq(&addr, expect, false));
assert(wireaddr_internal_eq(&addr, expect));

/* statictor address */
assert(parse_wireaddr_internal("statictor:127.0.0.1", &addr, DEFAULT_PORT, false, false, false, false, &err));
expect->itype = ADDR_INTERNAL_STATICTOR;
expect->u.torservice.port = DEFAULT_PORT;
memset(expect->u.torservice.blob, 0, sizeof(expect->u.torservice.blob));
strcpy((char *)expect->u.torservice.blob, STATIC_TOR_MAGIC_STRING);
assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9051, NULL, &err));
assert(wireaddr_internal_eq(&addr, expect, false));
assert(wireaddr_internal_eq(&addr, expect));

/* statictor address with port */
assert(parse_wireaddr_internal("statictor:127.0.0.1:9055", &addr, DEFAULT_PORT, false, false, false, false, &err));
expect->itype = ADDR_INTERNAL_STATICTOR;
expect->u.torservice.port = DEFAULT_PORT;
assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9055, NULL, &err));
assert(wireaddr_internal_eq(&addr, expect, false));
assert(wireaddr_internal_eq(&addr, expect));

/* statictor address with torport */
assert(parse_wireaddr_internal("statictor:127.0.0.1/torport=9055", &addr, DEFAULT_PORT, false, false, false, false, &err));
expect->itype = ADDR_INTERNAL_STATICTOR;
expect->u.torservice.port = 9055;
assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9051, NULL, &err));
assert(wireaddr_internal_eq(&addr, expect, false));
assert(wireaddr_internal_eq(&addr, expect));

/* statictor address with port and torport */
assert(parse_wireaddr_internal("statictor:127.0.0.1:9055/torport=10055", &addr, DEFAULT_PORT, false, false, false, false, &err));
expect->itype = ADDR_INTERNAL_STATICTOR;
expect->u.torservice.port = 10055;
assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9055, NULL, &err));
assert(wireaddr_internal_eq(&addr, expect, false));
assert(wireaddr_internal_eq(&addr, expect));

/* statictor address with port and torport and torblob */
assert(parse_wireaddr_internal("statictor:127.0.0.1:9055/torport=10055/torblob=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", &addr, DEFAULT_PORT, false, false, false, false, &err));
expect->itype = ADDR_INTERNAL_STATICTOR;
expect->u.torservice.port = 10055;
/* This is actually nul terminated */
memset(expect->u.torservice.blob, 'x', sizeof(expect->u.torservice.blob)-1);
assert(parse_wireaddr("127.0.0.1", &expect->u.torservice.address, 9055, NULL, &err));
assert(wireaddr_internal_eq(&addr, expect));

/* local socket path */
assert(parse_wireaddr_internal("/tmp/foo.sock", &addr, DEFAULT_PORT, false, false, false, false, &err));
expect->itype = ADDR_INTERNAL_SOCKNAME;
strcpy(expect->u.sockname, "/tmp/foo.sock");
assert(wireaddr_internal_eq(&addr, expect, false));
assert(wireaddr_internal_eq(&addr, expect));

/* Unresolved */
assert(!parse_wireaddr_internal("ozlabs.org", &addr, DEFAULT_PORT, false, false, false, false, &err));
Expand All @@ -244,7 +222,7 @@ int main(int argc, char *argv[])
expect->itype = ADDR_INTERNAL_FORPROXY;
strcpy(expect->u.unresolved.name, "ozlabs.org");
expect->u.unresolved.port = DEFAULT_PORT;
assert(wireaddr_internal_eq(&addr, expect, false));
assert(wireaddr_internal_eq(&addr, expect));

/* Unresolved with port */
assert(!parse_wireaddr_internal("ozlabs.org:1234", &addr, DEFAULT_PORT, false, false, false, false, &err));
Expand All @@ -253,7 +231,7 @@ int main(int argc, char *argv[])
expect->itype = ADDR_INTERNAL_FORPROXY;
strcpy(expect->u.unresolved.name, "ozlabs.org");
expect->u.unresolved.port = 1234;
assert(wireaddr_internal_eq(&addr, expect, false));
assert(wireaddr_internal_eq(&addr, expect));

tal_free(expect);
common_shutdown();
Expand Down
31 changes: 31 additions & 0 deletions common/wireaddr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,37 @@ bool parse_wireaddr(const char *arg, struct wireaddr *addr, u16 defport,
return res;
}

bool wireaddr_internal_eq(const struct wireaddr_internal *a,
const struct wireaddr_internal *b)
{
if (a->itype != b->itype)
return false;

switch (a->itype) {
case ADDR_INTERNAL_SOCKNAME:
return streq(a->u.sockname, b->u.sockname);
case ADDR_INTERNAL_ALLPROTO:
return a->u.port == b->u.port;
case ADDR_INTERNAL_STATICTOR:
if (!memeq(a->u.torservice.blob, sizeof(a->u.torservice.blob),
b->u.torservice.blob, sizeof(b->u.torservice.blob)))
return false;
/* fall thru */
case ADDR_INTERNAL_AUTOTOR:
if (!wireaddr_eq(&a->u.torservice.address,
&b->u.torservice.address))
return false;
return a->u.torservice.port == b->u.torservice.port;
case ADDR_INTERNAL_FORPROXY:
if (!streq(a->u.unresolved.name, b->u.unresolved.name))
return false;
return a->u.unresolved.port == b->u.unresolved.port;
case ADDR_INTERNAL_WIREADDR:
return wireaddr_eq(&a->u.wireaddr, &b->u.wireaddr);
}
abort();
}

bool parse_wireaddr_internal(const char *arg, struct wireaddr_internal *addr,
u16 port, bool wildcard_ok, bool dns_ok,
bool unresolved_ok, bool allow_deprecated,
Expand Down
3 changes: 3 additions & 0 deletions common/wireaddr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ struct wireaddr_internal {
char sockname[sizeof(((struct sockaddr_un *)0)->sun_path)];
} u;
};

bool wireaddr_internal_eq(const struct wireaddr_internal *a,
const struct wireaddr_internal *b);
bool parse_wireaddr_internal(const char *arg, struct wireaddr_internal *addr,
u16 port, bool wildcard_ok, bool dns_ok,
bool unresolved_ok, bool allow_deprecated,
Expand Down
31 changes: 12 additions & 19 deletions lightningd/options.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,14 +198,25 @@ static char *opt_add_addr_withtype(const char *arg,
deprecated_apis, &err_msg)) {
return tal_fmt(NULL, "Unable to parse address '%s': %s", arg, err_msg);
}

/* Sanity check for exact duplicates. */
for (size_t i = 0; i < tal_count(ld->proposed_wireaddr); i++) {
/* Only compare announce vs announce and bind vs bind */
if ((ld->proposed_listen_announce[i] & ala) == 0)
continue;

if (wireaddr_internal_eq(&ld->proposed_wireaddr[i], &wi))
return tal_fmt(NULL, "Duplicate %s address %s",
ala & ADDR_ANNOUNCE ? "announce" : "listen",
type_to_string(tmpctx, struct wireaddr_internal, &wi));
}
tal_arr_expand(&ld->proposed_wireaddr, wi);
return NULL;

}

static char *opt_add_announce_addr(const char *arg, struct lightningd *ld)
{
const struct wireaddr *wn;
size_t n = tal_count(ld->proposed_wireaddr);
char *err;

Expand All @@ -226,24 +237,6 @@ static char *opt_add_announce_addr(const char *arg, struct lightningd *ld)
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;
}

Expand Down
6 changes: 6 additions & 0 deletions tests/test_gossip.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -916,8 +916,11 @@ def test_gossip_addresses(node_factory, bitcoind):
l1 = node_factory.get_node(options={
'announce-addr': [
'[::]:3',
'[::]',
'127.0.0.1:2',
'127.0.0.1',
'vww6ybal4bd7szmgncyruucpgfkqahzddi37ktceo3ah7ngmcopnpyyd.onion',
'4acth47i6kxnvkewtm6q7ib2s3ufpo5sqbsnzjpbi7utijcltosqemad.onion:1234'
],
})
l2 = node_factory.get_node()
Expand All @@ -931,8 +934,11 @@ def test_gossip_addresses(node_factory, bitcoind):
nodes = l2.rpc.listnodes(l1.info['id'])['nodes']
assert len(nodes) == 1 and nodes[0]['addresses'] == [
{'type': 'ipv4', 'address': '127.0.0.1', 'port': 2},
{'type': 'ipv4', 'address': '127.0.0.1', 'port': 9735},
{'type': 'ipv6', 'address': '::', 'port': 3},
{'type': 'ipv6', 'address': '::', 'port': 9735},
{'type': 'torv3', 'address': 'vww6ybal4bd7szmgncyruucpgfkqahzddi37ktceo3ah7ngmcopnpyyd.onion', 'port': 9735},
{'type': 'torv3', 'address': '4acth47i6kxnvkewtm6q7ib2s3ufpo5sqbsnzjpbi7utijcltosqemad.onion', 'port': 1234},
]


Expand Down

0 comments on commit 9d18180

Please sign in to comment.