From 10c503b4b471c5db536e6344bf293ef4546f03ed Mon Sep 17 00:00:00 2001
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Fri, 14 Jun 2019 13:00:56 +0930
Subject: [PATCH] gossip_store: clean up a truncated store.

We might have channel_announcements which have no channel_update: normally
these don't get written into the store until there is one, but if the
store was truncated it can happen.  We then get upset on compaction, since
we don't have an in-memory representation of the channel_announcement.

Similarly, we leave the node_announcement pending until after that
channel_announcement, leading to a similar case.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 gossipd/gossip_store.c | 51 +++++++++++++++++++++++++++++++++++-------
 gossipd/routing.c      | 40 +++++++++++++++++++++++++++++++++
 gossipd/routing.h      |  8 +++++++
 tests/test_gossip.py   |  1 -
 4 files changed, 91 insertions(+), 9 deletions(-)

diff --git a/gossipd/gossip_store.c b/gossipd/gossip_store.c
index 0ba89aa92..4b5534401 100644
--- a/gossipd/gossip_store.c
+++ b/gossipd/gossip_store.c
@@ -508,6 +508,48 @@ 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,
+				    u32 chan_ann_off)
+{
+	size_t num;
+	u32 index;
+
+	/* channel_announce with no channel_amount. */
+	if (chan_ann_off) {
+		status_unusual("Deleting un-amounted channel_announcement @%u",
+			       chan_ann_off);
+		delete_by_index(gs, chan_ann_off, WIRE_CHANNEL_ANNOUNCEMENT);
+	}
+
+	num = 0;
+	while ((index = remove_unfinalized_node_announce(rstate)) != 0) {
+		delete_by_index(gs, index, WIRE_NODE_ANNOUNCEMENT);
+		num++;
+	}
+	if (num)
+		status_unusual("Deleted %zu unfinalized node_announcements",
+			       num);
+
+	num = 0;
+	while ((index = remove_unupdated_channel_announce(rstate)) != 0) {
+		delete_by_index(gs, index, WIRE_CHANNEL_ANNOUNCEMENT);
+		num++;
+	}
+	if (num)
+		status_unusual("Deleted %zu unupdated channel_announcements",
+			       num);
+}
+
 bool gossip_store_load(struct routing_state *rstate, struct gossip_store *gs)
 {
 	struct gossip_hdr hdr;
@@ -635,14 +677,7 @@ truncate_nomsg:
 	contents_ok = false;
 out:
 	gs->writable = true;
-	/* If we ever truncated, we might have a dangling channel_announce */
-	if (chan_ann) {
-		struct broadcastable bcast;
-		bcast.index = chan_ann_off;
-		status_unusual("Deleting un-updated channel_announcement @%"
-			       PRIu64, chan_ann_off);
-		gossip_store_delete(gs, &bcast, WIRE_CHANNEL_ANNOUNCEMENT);
-	}
+	cleanup_truncated_store(rstate, gs, chan_ann ? chan_ann_off : 0);
 	status_trace("total store load time: %"PRIu64" msec",
 		     time_to_msec(time_between(time_now(), start)));
 	status_trace("gossip_store: Read %zu/%zu/%zu/%zu cannounce/cupdate/nannounce/cdelete from store (%zu deleted) in %"PRIu64" bytes",
diff --git a/gossipd/routing.c b/gossipd/routing.c
index 68e7f958e..316268c27 100644
--- a/gossipd/routing.c
+++ b/gossipd/routing.c
@@ -2581,3 +2581,43 @@ struct timeabs gossip_time_now(const struct routing_state *rstate)
 #endif
 	return time_now();
 }
+
+/* gossip_store wants to delete any dangling node_announcement msgs */
+u32 remove_unfinalized_node_announce(struct routing_state *rstate)
+{
+	/* We're only interested in node_announcement we caught. */
+	for (;;) {
+		struct pending_node_announce *pna;
+		struct pending_node_map_iter it;
+
+		pna = pending_node_map_first(rstate->pending_node_map, &it);
+		if (!pna)
+			return 0;
+
+		/* This will be deleted by the associated unupdated_channel; just
+		 * remove from map for now. */
+		pending_node_map_del(rstate->pending_node_map, pna);
+		if (!pna->node_announcement)
+			continue;
+
+		assert(pna->index);
+		return pna->index;
+	}
+}
+
+/* gossip_store wants to delete any dangling channel_announcement msgs */
+u32 remove_unupdated_channel_announce(struct routing_state *rstate)
+{
+	struct unupdated_channel *uc;
+	u64 index;
+
+	uc = uintmap_first(&rstate->unupdated_chanmap, &index);
+	if (!uc)
+		return 0;
+
+	assert(uc->index);
+	index = uc->index;
+
+	tal_free(uc);
+	return index;
+}
diff --git a/gossipd/routing.h b/gossipd/routing.h
index 149f6ca8a..392ac4864 100644
--- a/gossipd/routing.h
+++ b/gossipd/routing.h
@@ -437,4 +437,12 @@ struct wireaddr *read_addresses(const tal_t *ctx, const u8 *ser);
 void remove_channel_from_store(struct routing_state *rstate,
 			       struct chan *chan);
 
+/* gossip_store wants to delete any dangling entries immediately after
+ * load; return 0 if no more, otherwise index into store.
+ *
+ * Must call remove_unfinalized_node_announce first, because removing
+ * unupdated channels may delete associatd node_announcements. */
+u32 remove_unfinalized_node_announce(struct routing_state *rstate);
+u32 remove_unupdated_channel_announce(struct routing_state *rstate);
+
 #endif /* LIGHTNING_GOSSIPD_ROUTING_H */
diff --git a/tests/test_gossip.py b/tests/test_gossip.py
index 48701d235..6966778da 100644
--- a/tests/test_gossip.py
+++ b/tests/test_gossip.py
@@ -1234,7 +1234,6 @@ def test_gossip_store_compact_restart(node_factory, bitcoind):
     l2.rpc.call('dev-compact-gossip-store')
 
 
-@pytest.mark.xfail(strict=True)
 @unittest.skipIf(not DEVELOPER, "need dev-compact-gossip-store")
 def test_gossip_store_load_no_channel_update(node_factory):
     """Make sure we can read truncated gossip store with a channel_announcement and no channel_update"""