gossipd: delete channel_amount from the store when we delete channel_announcement.

Otherwise we slowly build up cruft: compaction simply moves them since
they're not deleted.

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

gossipd/gossip_store.c

@@ -398,27 +398,24 @@ u64 gossip_store_add_private_update(struct gossip_store *gs, const u8 *update)
 	return gossip_store_add(gs, pupdate, 0, NULL);
 }
 
-void gossip_store_delete(struct gossip_store *gs,
-			 struct broadcastable *bcast,
-			 int type)
+/* Returns index of following entry. */
+static u32 delete_by_index(struct gossip_store *gs, u32 index, int type)
 {
 	beint32_t belen;
 	int flags;
 
-	if (!bcast->index)
-		return;
-
 	/* Should never get here during loading! */
 	assert(gs->writable);
 
 #if DEVELOPER
-	const u8 *msg = gossip_store_get(tmpctx, gs, bcast->index);
+	const u8 *msg = gossip_store_get(tmpctx, gs, index);
 	assert(fromwire_peektype(msg) == type);
 #endif
-	if (pread(gs->fd, &belen, sizeof(belen), bcast->index) != sizeof(belen))
+
+	if (pread(gs->fd, &belen, sizeof(belen), index) != sizeof(belen))
 		status_failed(STATUS_FAIL_INTERNAL_ERROR,
 			      "Failed reading len to delete @%u: %s",
-			      bcast->index, strerror(errno));
+			      index, strerror(errno));
 
 	assert((be32_to_cpu(belen) & GOSSIP_STORE_LEN_DELETED_BIT) == 0);
 	belen |= cpu_to_be32(GOSSIP_STORE_LEN_DELETED_BIT);
@@ -433,16 +430,36 @@ void gossip_store_delete(struct gossip_store *gs,
 	 */
 	flags = fcntl(gs->fd, F_GETFL);
 	fcntl(gs->fd, F_SETFL, flags & ~O_APPEND);
-	if (pwrite(gs->fd, &belen, sizeof(belen), bcast->index) != sizeof(belen))
+	if (pwrite(gs->fd, &belen, sizeof(belen), index) != sizeof(belen))
 		status_failed(STATUS_FAIL_INTERNAL_ERROR,
 			      "Failed writing len to delete @%u: %s",
-			      bcast->index, strerror(errno));
+			      index, strerror(errno));
 	fcntl(gs->fd, F_SETFL, flags);
 	gs->deleted++;
 
+	return index + sizeof(struct gossip_hdr)
+		+ (be32_to_cpu(belen) & ~GOSSIP_STORE_LEN_DELETED_BIT);
+}
+
+void gossip_store_delete(struct gossip_store *gs,
+			 struct broadcastable *bcast,
+			 int type)
+{
+	u32 next_index;
+
+	if (!bcast->index)
+		return;
+
+	next_index = delete_by_index(gs, bcast->index, type);
+
 	/* Reset index. */
 	bcast->index = 0;
 
+	/* For a channel_announcement, we need to delete amount too */
+	if (type == WIRE_CHANNEL_ANNOUNCEMENT)
+		delete_by_index(gs, next_index,
+				WIRE_GOSSIP_STORE_CHANNEL_AMOUNT);
+
 	gossip_store_maybe_compact(gs);
 }
 
@@ -508,14 +525,6 @@ int gossip_store_readonly_fd(struct gossip_store *gs)
 	return fd;
 }
 
-static void delete_by_index(struct gossip_store *gs, u32 index, int type)
-{
-	/* We make a fake broadcastable for this corner case. */
-	struct broadcastable bcast;
-	bcast.index = index;
-	gossip_store_delete(gs, &bcast, type);
-}
-
 /* If we ever truncated, we might have a dangling entries. */
 static void cleanup_truncated_store(struct routing_state *rstate,
 				    struct gossip_store *gs,
@@ -542,7 +551,11 @@ static void cleanup_truncated_store(struct routing_state *rstate,
 
 	num = 0;
 	while ((index = remove_unupdated_channel_announce(rstate)) != 0) {
-		delete_by_index(gs, index, WIRE_CHANNEL_ANNOUNCEMENT);
+		u32 next;
+
+		/* Delete announcement and channel amount, too */
+		next = delete_by_index(gs, index, WIRE_CHANNEL_ANNOUNCEMENT);
+		delete_by_index(gs, next, WIRE_GOSSIP_STORE_CHANNEL_AMOUNT);
 		num++;
 	}
 	if (num)

tests/test_gossip.py

@@ -945,7 +945,7 @@ def test_gossip_store_load_amount_truncated(node_factory):
 
     l1.start()
     # May preceed the Started msg waited for in 'start'.
-    wait_for(lambda: l1.daemon.is_in_log(r'Deleting un-updated channel_announcement @1'))
+    wait_for(lambda: l1.daemon.is_in_log(r'Deleting un-amounted channel_announcement @1'))
     wait_for(lambda: l1.daemon.is_in_log(r'gossip_store: Read 0/0/0/0 cannounce/cupdate/nannounce/cdelete from store \(1 deleted\) in 445 bytes'))
     assert not l1.daemon.is_in_log('gossip_store.*truncating')
 
@@ -1263,11 +1263,5 @@ def test_gossip_store_load_no_channel_update(node_factory):
     # This should actually result in an empty store.
     l1.rpc.call('dev-compact-gossip-store')
 
-    # FIXME: We leave channel_amount in the store!
     with open(os.path.join(l1.daemon.lightning_dir, 'gossip_store'), "rb") as f:
-        assert bytearray(f.read()) == bytearray.fromhex("07"
-                                                        "0000000a"  # len
-                                                        "99dc98b4"  # csum
-                                                        "00000000"  # timestamp
-                                                        "1005"      # WIRE_GOSSIP_STORE_CHANNEL_AMOUNT
-                                                        "0000000001000000")
+        assert bytearray(f.read()) == bytearray.fromhex("07")