From 2996dc500463179fdef4563d982d0487d0e5c5bf Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 24 Jan 2024 15:03:04 +1030 Subject: [PATCH] lightingd: corrections from Alex Myers's review. Signed-off-by: Rusty Russell --- lightningd/channel_gossip.c | 2 +- lightningd/channel_gossip.h | 2 +- lightningd/gossip_generation.c | 6 +++++- tests/test_connection.py | 10 +++++----- tests/test_gossip.py | 2 +- 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/lightningd/channel_gossip.c b/lightningd/channel_gossip.c index 2fe213eb0625..16817798e1f1 100644 --- a/lightningd/channel_gossip.c +++ b/lightningd/channel_gossip.c @@ -945,7 +945,7 @@ const u8 *channel_gossip_update_for_error(const tal_t *ctx, * - MUST set the `short_channel_id` to either an `alias` it has * received from the peer, or the real channel `short_channel_id`. */ -/* But we used to get this wrong! Sot this is the only place where we +/* But we used to get this wrong! So this is the only place where we * look up by *remote* id. It's not unique, but it is unique for a * specific peer. */ static struct channel *lookup_by_peer_remote_alias(struct lightningd *ld, diff --git a/lightningd/channel_gossip.h b/lightningd/channel_gossip.h index 9170a98ade55..0a81089dea14 100644 --- a/lightningd/channel_gossip.h +++ b/lightningd/channel_gossip.h @@ -49,7 +49,7 @@ void channel_gossip_channel_reestablished(struct channel *channel); /* Peer has disconnected */ void channel_gossip_channel_disconnect(struct channel *channel); -/* Gossipd sent us this channel_update about the peer's side of the channle */ +/* Gossipd sent us this channel_update about the peer's side of the channel */ void channel_gossip_set_remote_update(struct lightningd *ld, const struct peer_update *update TAKES, const struct node_id *source); diff --git a/lightningd/gossip_generation.c b/lightningd/gossip_generation.c index 1d20bccc81f0..afe09a54f79e 100644 --- a/lightningd/gossip_generation.c +++ b/lightningd/gossip_generation.c @@ -125,6 +125,10 @@ u8 *unsigned_channel_update(const tal_t *ctx, /* Make sure timestamp changes! */ timestamp = time_now().ts.tv_sec; + /* FIXME: @endothermicdev points out that our clock could be + * wrong once, and now we'll keep producing future timestamps. + * We could sanity check that old_timestamp is within 2 weeks and + * discard? */ if (old_timestamp && timestamp <= *old_timestamp) timestamp = *old_timestamp + 1; @@ -347,7 +351,7 @@ static const struct wireaddr *gather_addresses(const tal_t *ctx, { struct wireaddr *addrs; - /* Note: If no announceable, tal_dup_talarr returns NULL! */ + /* Note: If ld->announceable is NULL, tal_dup_talarr returns NULL! */ addrs = tal_dup_talarr(ctx, struct wireaddr, ld->announceable); if (!addrs) addrs = tal_arr(ctx, struct wireaddr, 0); diff --git a/tests/test_connection.py b/tests/test_connection.py index e6503bdb4ed0..9dc58c037af1 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -111,7 +111,7 @@ def test_remote_addr(node_factory, bitcoind): def_port = default_ln_port(l2.info["network"]) - # when we restart l1 with a channel and reconnect, node_annoucement update + # when we restart l1 with a channel and reconnect, node_announcement update # must not yet be send as we need the same `remote_addr` confirmed from a # another peer we have a channel with. # Note: In this state l2 stores remote_addr as reported by l1 @@ -125,7 +125,7 @@ def test_remote_addr(node_factory, bitcoind): assert not l2.daemon.is_in_log("Update our node_announcement for discovered address: 127.0.0.1:{}".format(def_port)) assert len(l2.rpc.getinfo()['address']) == 0 - # connect second node. This will trigger `node_annoucement` update. + # connect second node. This will trigger `node_announcement` update. l2.rpc.connect(l3.info['id'], 'localhost', l3.port) l2.daemon.wait_for_log("Peer says it sees our address as: 127.0.0.1:[0-9]{5}") l2.daemon.wait_for_log("Update our node_announcement for discovered address: 127.0.0.1:{}".format(def_port)) @@ -146,7 +146,7 @@ def test_remote_addr(node_factory, bitcoind): def test_remote_addr_disabled(node_factory, bitcoind): - """Simply tests that IP address discovery annoucements can be turned off + """Simply tests that IP address discovery announcements can be turned off We perform logic tests on L2, setup: l1 --> [l2] <-- l3 @@ -179,7 +179,7 @@ def test_remote_addr_disabled(node_factory, bitcoind): l2.daemon.wait_for_log(f"{l3.info['id']}.*Already have funding locked in") # if ip discovery would have been enabled, we would have send an updated - # node_annoucement by now. Check we didn't... + # node_announcement by now. Check we didn't... bitcoind.generate_block(6) # ugly, but we need to wait for gossip... assert not l2.daemon.is_in_log("Update our node_announcement for discovered address") @@ -225,7 +225,7 @@ def test_remote_addr_port(node_factory, bitcoind): l2.rpc.connect(l3.info['id'], 'localhost', l3.port) # if ip discovery would have been enabled, we would have send an updated - # node_annoucement by now. Check we didn't... + # node_announcement by now. Check we didn't... l2.daemon.wait_for_logs(["Already have funding locked in", "Update our node_announcement for discovered address"]) info = l2.rpc.getinfo() diff --git a/tests/test_gossip.py b/tests/test_gossip.py index d4142c329be4..552c118f5bd4 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -178,7 +178,7 @@ def test_announce_dns_suppressed(node_factory, bitcoind): def test_announce_and_connect_via_dns(node_factory, bitcoind): - """ Test that DNS annoucements propagate and can be used when connecting. + """ Test that DNS announcements propagate and can be used when connecting. - First node announces only a FQDN like 'localhost.localdomain'. - Second node gets a channel with first node.