lightningd: don't generate node_announcements with identical timestamps.

This caused a flake in test_gossip_lease_rates, since the peer would ignore the node_announcement due to duplicate timestamps! Signed-off-by: Rusty Russell <[email protected]>
ElementsProject · Jan 31, 2024 · c5e4307 · c5e4307
1 parent fffb765
commit c5e4307
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 9 deletions.
diff --git a/lightningd/channel_gossip.c b/lightningd/channel_gossip.c
@@ -1065,7 +1065,7 @@ void channel_gossip_node_announce(struct lightningd *ld)
 	if (!gossipd_init_done)
 		return;
 
-	nannounce = unsigned_node_announcement(tmpctx, ld);
+	nannounce = unsigned_node_announcement(tmpctx, ld, ld->node_announcement);
 
 	/* Don't bother with duplicates */
 	if (ld->node_announcement

diff --git a/lightningd/gossip_generation.c b/lightningd/gossip_generation.c
@@ -304,6 +304,39 @@ static void get_nannounce_parts(const u8 *node_announcement,
 		parts[2] = NULL;
 }
 
+/* Get timestamp of a (valid!) node_announcement  */
+static u32 get_nannounce_timestamp(const u8 *node_announcement)
+{
+	const u8 *p;
+	u16 flen;
+	size_t len;
+	u32 timestamp;
+
+	/* BOLT #7:
+	 *
+	 * 1. type: 257 (`node_announcement`)
+	 * 2. data:
+	 *    * [`signature`:`signature`]
+	 *    * [`u16`:`flen`]
+	 *    * [`flen*byte`:`features`]
+	 *    * [`u32`:`timestamp`]
+	 *...
+	 */
+	len = tal_count(node_announcement);
+	p = node_announcement;
+
+	/* Note: 2 bytes for `type` field */
+	fromwire_u16(&p, &len);
+	fromwire(&p, &len, NULL, 64);
+	flen = fromwire_u16(&p, &len);
+	fromwire(&p, &len, NULL, flen);
+
+	timestamp = fromwire_u32(&p, &len);
+	assert(p != NULL);
+
+	return timestamp;
+}
+
 /* Is nann1 same as nann2 (not sigs and timestamps)? */
 bool node_announcement_same(const u8 *nann1, const u8 *nann2)
 {
@@ -379,16 +412,24 @@ static const struct wireaddr *gather_addresses(const tal_t *ctx,
 }
 
 u8 *unsigned_node_announcement(const tal_t *ctx,
-			       struct lightningd *ld)
+			       struct lightningd *ld,
+			       const u8 *prev)
 {
 	secp256k1_ecdsa_signature sig;
 	const struct wireaddr *addrs;
+	u32 timestamp = time_now().ts.tv_sec;
 
 	addrs = gather_addresses(tmpctx, ld);
+	/* Even if we're quick, don't duplicate timestamps! */
+	if (prev) {
+		u32 old_timestamp = get_nannounce_timestamp(prev);
+		if (timestamp <= old_timestamp)
+			timestamp = old_timestamp + 1;
+	}
 
 	memset(&sig, 0, sizeof(sig));
 	return create_nannounce(tmpctx, ld, &sig,
-				addrs, time_now().ts.tv_sec,
+				addrs, timestamp,
 				ld->lease_rates);
 }
 

diff --git a/lightningd/gossip_generation.h b/lightningd/gossip_generation.h
@@ -81,8 +81,11 @@ const char *check_announce_sigs(const struct channel *channel,
  * unsigned_node_announcement: create a current unsigned node announcement.
  * @ctx: the context to allocate return from
  * @ld: the lightningd struct.
+ * @prev: optional previous announcement (to ensure we increase timestamp!)
  */
-u8 *unsigned_node_announcement(const tal_t *ctx, struct lightningd *ld);
+u8 *unsigned_node_announcement(const tal_t *ctx,
+			       struct lightningd *ld,
+			       const u8 *prev);
 
 /**
  * add_node_announcement_sig: apply the signature to the node announcement

diff --git a/tests/test_gossip.py b/tests/test_gossip.py
@@ -1073,10 +1073,6 @@ def test_gossip_lease_rates(node_factory, bitcoind):
                   'channel-fee-max-proportional-thousandths': 200}
     l1, l2 = node_factory.get_nodes(2, opts=[lease_opts, {}])
 
-    # These logs happen during startup, start looking from the beginning
-    l1.daemon.logsearch_start = 0
-    l2.daemon.logsearch_start = 0
-
     rates = l1.rpc.call('funderupdate')
     assert rates['channel_fee_max_base_msat'] == Millisatoshi('500000msat')
     assert rates['channel_fee_max_proportional_thousandths'] == 200

diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c
@@ -1171,7 +1171,9 @@ u8 *unsigned_channel_update(const tal_t *ctx UNNEEDED,
 			    bool enabled UNNEEDED)
 { fprintf(stderr, "unsigned_channel_update called!\n"); abort(); }
 /* Generated stub for unsigned_node_announcement */
-u8 *unsigned_node_announcement(const tal_t *ctx UNNEEDED, struct lightningd *ld UNNEEDED)
+u8 *unsigned_node_announcement(const tal_t *ctx UNNEEDED,
+			       struct lightningd *ld UNNEEDED,
+			       const u8 *prev UNNEEDED)
 { fprintf(stderr, "unsigned_node_announcement called!\n"); abort(); }
 /* Generated stub for unwrap_onionreply */
 u8 *unwrap_onionreply(const tal_t *ctx UNNEEDED,