gossipd: take txout failure cache out of routing.c

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

gossipd/Makefile

@@ -7,6 +7,7 @@ GOSSIPD_HEADERS_WSRC := gossipd/gossipd_wiregen.h \
 	gossipd/gossip_store.h				\
 	gossipd/queries.h				\
 	gossipd/routing.h				\
+	gossipd/txout_failures.h			\
 	gossipd/seeker.h
 GOSSIPD_HEADERS := $(GOSSIPD_HEADERS_WSRC) gossipd/broadcast.h
 

gossipd/routing.c

@@ -16,6 +16,7 @@
 #include <gossipd/gossipd_wiregen.h>
 #include <gossipd/queries.h>
 #include <gossipd/routing.h>
+#include <gossipd/txout_failures.h>
 
 #ifndef SUPERVERBOSE
 #define SUPERVERBOSE(...)
@@ -248,43 +249,6 @@ static void memleak_help_routing_tables(struct htable *memtable,
 	}
 }
 
-/* Once an hour, or at 10000 entries, we expire old ones */
-static void txout_failure_age(struct routing_state *rstate)
-{
-	uintmap_clear(&rstate->txout_failures_old);
-	rstate->txout_failures_old = rstate->txout_failures;
-	uintmap_init(&rstate->txout_failures);
-	rstate->num_txout_failures = 0;
-
-	rstate->txout_failure_timer = new_reltimer(&rstate->daemon->timers,
-						   rstate, time_from_sec(3600),
-						   txout_failure_age, rstate);
-}
-
-static void add_to_txout_failures(struct routing_state *rstate,
-				  const struct short_channel_id *scid)
-{
-	if (uintmap_add(&rstate->txout_failures, scid->u64, true)
-	    && ++rstate->num_txout_failures == 10000) {
-		tal_free(rstate->txout_failure_timer);
-		txout_failure_age(rstate);
-	}
-}
-
-static bool in_txout_failures(struct routing_state *rstate,
-			      const struct short_channel_id *scid)
-{
-	if (uintmap_get(&rstate->txout_failures, scid->u64))
-		return true;
-
-	/* If we were going to expire it, we no longer are. */
-	if (uintmap_get(&rstate->txout_failures_old, scid->u64)) {
-		add_to_txout_failures(rstate, scid);
-		return true;
-	}
-	return false;
-}
-
 struct routing_state *new_routing_state(const tal_t *ctx,
 					struct daemon *daemon,
 					const u32 *dev_gossip_time TAKES,
@@ -303,10 +267,7 @@ struct routing_state *new_routing_state(const tal_t *ctx,
 
 	uintmap_init(&rstate->chanmap);
 	uintmap_init(&rstate->unupdated_chanmap);
-	rstate->num_txout_failures = 0;
-	uintmap_init(&rstate->txout_failures);
-	uintmap_init(&rstate->txout_failures_old);
-	txout_failure_age(rstate);
+	rstate->txf = txout_failures_new(rstate, rstate->daemon);
 	rstate->pending_node_map = tal(ctx, struct pending_node_map);
 	pending_node_map_init(rstate->pending_node_map);
 
@@ -974,7 +935,7 @@ u8 *handle_channel_announcement(struct routing_state *rstate,
 
 	/* If a prior txout lookup failed there is little point it trying
 	 * again. Just drop the announcement and walk away whistling. */
-	if (in_txout_failures(rstate, &pending->short_channel_id)) {
+	if (in_txout_failures(rstate->txf, pending->short_channel_id)) {
 		SUPERVERBOSE(
 		    "Ignoring channel_announcement of %s due to a prior txout "
 		    "query failure. The channel was likely closed on-chain.",
@@ -1147,7 +1108,7 @@ bool handle_pending_cannouncement(struct daemon *daemon,
 						 struct short_channel_id,
 						 scid));
 		tal_free(pending);
-		add_to_txout_failures(rstate, scid);
+		txout_failures_add(rstate->txf, *scid);
 		return false;
 	}
 
@@ -1665,7 +1626,7 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update TAKES,
 	/* If we dropped the matching announcement for this channel due to the
 	 * txout query failing, don't report failure, it's just too noisy on
 	 * mainnet */
-	if (in_txout_failures(rstate, &short_channel_id))
+	if (in_txout_failures(rstate->txf, short_channel_id))
 		return NULL;
 
 	/* If we have an unvalidated channel, just queue on that */
@@ -2213,7 +2174,7 @@ static void channel_spent(struct routing_state *rstate,
 		     type_to_string(tmpctx, struct short_channel_id,
 				    &chan->scid));
 	/* Suppress any now-obsolete updates/announcements */
-	add_to_txout_failures(rstate, &chan->scid);
+	txout_failures_add(rstate->txf, chan->scid);
 	remove_channel_from_store(rstate, chan);
 	/* Freeing is sufficient since everything else is allocated off
 	 * of the channel and this takes care of unregistering

gossipd/routing.h

@@ -198,9 +198,7 @@ struct routing_state {
 
 	/* Cache for txout queries that failed. Allows us to skip failed
 	 * checks if we get another announcement for the same scid. */
-	size_t num_txout_failures;
-	UINTMAP(bool) txout_failures, txout_failures_old;
-	struct oneshot *txout_failure_timer;
+	struct txout_failures *txf;
 
 	/* Highest timestamp of gossip we accepted (before now) */
 	u32 last_timestamp;

gossipd/test/run-check_channel_announcement.c

@@ -89,6 +89,10 @@ void gossip_store_mark_dying(struct gossip_store *gs UNNEEDED,
 /* Generated stub for gossip_store_new */
 struct gossip_store *gossip_store_new(struct routing_state *rstate UNNEEDED)
 { fprintf(stderr, "gossip_store_new called!\n"); abort(); }
+/* Generated stub for in_txout_failures */
+bool in_txout_failures(struct txout_failures *txf UNNEEDED,
+		       const struct short_channel_id scid UNNEEDED)
+{ fprintf(stderr, "in_txout_failures called!\n"); abort(); }
 /* Generated stub for memleak_add_helper_ */
 void memleak_add_helper_(const tal_t *p UNNEEDED, void (*cb)(struct htable *memtable UNNEEDED,
 						    const tal_t *)){ }
@@ -98,12 +102,6 @@ void memleak_scan_htable(struct htable *memtable UNNEEDED, const struct htable *
 /* Generated stub for memleak_scan_intmap_ */
 void memleak_scan_intmap_(struct htable *memtable UNNEEDED, const struct intmap *m UNNEEDED)
 { fprintf(stderr, "memleak_scan_intmap_ called!\n"); abort(); }
-/* Generated stub for new_reltimer_ */
-struct oneshot *new_reltimer_(struct timers *timers UNNEEDED,
-			      const tal_t *ctx UNNEEDED,
-			      struct timerel expire UNNEEDED,
-			      void (*cb)(void *) UNNEEDED, void *arg UNNEEDED)
-{ fprintf(stderr, "new_reltimer_ called!\n"); abort(); }
 /* Generated stub for notleak_ */
 void *notleak_(void *ptr UNNEEDED, bool plus_children UNNEEDED)
 { fprintf(stderr, "notleak_ called!\n"); abort(); }
@@ -121,6 +119,13 @@ void status_fmt(enum log_level level UNNEEDED,
 /* Generated stub for towire_gossipd_remote_channel_update */
 u8 *towire_gossipd_remote_channel_update(const tal_t *ctx UNNEEDED, const struct node_id *source_node UNNEEDED, const struct peer_update *peer_update UNNEEDED)
 { fprintf(stderr, "towire_gossipd_remote_channel_update called!\n"); abort(); }
+/* Generated stub for txout_failures_add */
+void txout_failures_add(struct txout_failures *txf UNNEEDED,
+			const struct short_channel_id scid UNNEEDED)
+{ fprintf(stderr, "txout_failures_add called!\n"); abort(); }
+/* Generated stub for txout_failures_new */
+struct txout_failures *txout_failures_new(const tal_t *ctx UNNEEDED, struct daemon *daemon UNNEEDED)
+{ fprintf(stderr, "txout_failures_new called!\n"); abort(); }
 /* AUTOGENERATED MOCKS END */
 
 int main(int argc, char *argv[])

gossipd/test/run-txout_failure.c

@@ -1,12 +1,15 @@
 #include "config.h"
-#include "../routing.c"
+#include "../txout_failures.c"
 #include "../common/timeout.c"
 #include <common/blinding.h>
 #include <common/channel_type.h>
+#include <common/daemon_conn.h>
 #include <common/ecdh.h>
 #include <common/json_stream.h>
 #include <common/onionreply.h>
 #include <common/setup.h>
+#include <gossipd/gossip_store.h>
+#include <gossipd/queries.h>
 #include <stdio.h>
 
 /* AUTOGENERATED MOCKS START */
@@ -25,73 +28,6 @@ bool blinding_next_pubkey(const struct pubkey *pk UNNEEDED,
 			  const struct sha256 *h UNNEEDED,
 			  struct pubkey *next UNNEEDED)
 { fprintf(stderr, "blinding_next_pubkey called!\n"); abort(); }
-/* Generated stub for daemon_conn_send */
-void daemon_conn_send(struct daemon_conn *dc UNNEEDED, const u8 *msg UNNEEDED)
-{ fprintf(stderr, "daemon_conn_send called!\n"); abort(); }
-/* Generated stub for get_cupdate_parts */
-void get_cupdate_parts(const u8 *channel_update UNNEEDED,
-		       const u8 *parts[2] UNNEEDED,
-		       size_t sizes[2])
-{ fprintf(stderr, "get_cupdate_parts called!\n"); abort(); }
-/* Generated stub for gossip_store_add */
-u64 gossip_store_add(struct gossip_store *gs UNNEEDED, const u8 *gossip_msg UNNEEDED,
-		     u32 timestamp UNNEEDED, bool zombie UNNEEDED, bool spam UNNEEDED, bool dying UNNEEDED,
-		     const u8 *addendum UNNEEDED)
-{ fprintf(stderr, "gossip_store_add called!\n"); abort(); }
-/* Generated stub for gossip_store_delete */
-void gossip_store_delete(struct gossip_store *gs UNNEEDED,
-			 struct broadcastable *bcast UNNEEDED,
-			 int type UNNEEDED)
-{ fprintf(stderr, "gossip_store_delete called!\n"); abort(); }
-/* Generated stub for gossip_store_get */
-const u8 *gossip_store_get(const tal_t *ctx UNNEEDED,
-			   struct gossip_store *gs UNNEEDED,
-			   u64 offset UNNEEDED)
-{ fprintf(stderr, "gossip_store_get called!\n"); abort(); }
-/* Generated stub for gossip_store_mark_channel_deleted */
-void gossip_store_mark_channel_deleted(struct gossip_store *gs UNNEEDED,
-				       const struct short_channel_id *scid UNNEEDED)
-{ fprintf(stderr, "gossip_store_mark_channel_deleted called!\n"); abort(); }
-/* Generated stub for gossip_store_mark_dying */
-void gossip_store_mark_dying(struct gossip_store *gs UNNEEDED,
-			     const struct broadcastable *bcast UNNEEDED,
-			     int type UNNEEDED)
-{ fprintf(stderr, "gossip_store_mark_dying called!\n"); abort(); }
-/* Generated stub for memleak_add_helper_ */
-void memleak_add_helper_(const tal_t *p UNNEEDED, void (*cb)(struct htable *memtable UNNEEDED,
-						    const tal_t *)){ }
-/* Generated stub for memleak_scan_htable */
-void memleak_scan_htable(struct htable *memtable UNNEEDED, const struct htable *ht UNNEEDED)
-{ fprintf(stderr, "memleak_scan_htable called!\n"); abort(); }
-/* Generated stub for memleak_scan_intmap_ */
-void memleak_scan_intmap_(struct htable *memtable UNNEEDED, const struct intmap *m UNNEEDED)
-{ fprintf(stderr, "memleak_scan_intmap_ called!\n"); abort(); }
-/* Generated stub for notleak_ */
-void *notleak_(void *ptr UNNEEDED, bool plus_children UNNEEDED)
-{ fprintf(stderr, "notleak_ called!\n"); abort(); }
-/* Generated stub for peer_supplied_good_gossip */
-void peer_supplied_good_gossip(struct daemon *daemon UNNEEDED,
-			       const struct node_id *source_peer UNNEEDED,
-			       size_t amount UNNEEDED)
-{ fprintf(stderr, "peer_supplied_good_gossip called!\n"); abort(); }
-/* Generated stub for sanitize_error */
-char *sanitize_error(const tal_t *ctx UNNEEDED, const u8 *errmsg UNNEEDED,
-		     struct channel_id *channel_id UNNEEDED)
-{ fprintf(stderr, "sanitize_error called!\n"); abort(); }
-/* Generated stub for status_fmt */
-void status_fmt(enum log_level level UNNEEDED,
-		const struct node_id *peer UNNEEDED,
-		const char *fmt UNNEEDED, ...)
-
-{ fprintf(stderr, "status_fmt called!\n"); abort(); }
-/* Generated stub for towire_gossipd_remote_channel_update */
-u8 *towire_gossipd_remote_channel_update(const tal_t *ctx UNNEEDED, const struct node_id *source_node UNNEEDED, const struct peer_update *peer_update UNNEEDED)
-{ fprintf(stderr, "towire_gossipd_remote_channel_update called!\n"); abort(); }
-/* Generated stub for towire_warningfmt */
-u8 *towire_warningfmt(const tal_t *ctx UNNEEDED,
-		      const struct channel_id *channel UNNEEDED,
-		      const char *fmt UNNEEDED, ...)
-{ fprintf(stderr, "towire_warningfmt called!\n"); abort(); }
 /* AUTOGENERATED MOCKS END */
 
 /* NOOP stub for gossip_store_new */
@@ -102,7 +38,7 @@ struct gossip_store *gossip_store_new(struct routing_state *rstate UNNEEDED)
 
 int main(int argc, char *argv[])
 {
-	struct routing_state *rstate;
+	struct txout_failures *txf;
 	struct timer *t;
 	struct short_channel_id scid1, scid2;
 	struct daemon *daemon;
@@ -111,22 +47,22 @@ int main(int argc, char *argv[])
 
 	daemon = tal(tmpctx, struct daemon);
 	timers_init(&daemon->timers, time_mono());
-	rstate = new_routing_state(tmpctx, daemon, NULL, false, false);
+	txf = txout_failures_new(tmpctx, daemon);
 
 	scid1.u64 = 100;
 	scid2.u64 = 200;
-	assert(!in_txout_failures(rstate, &scid1));
-	assert(!in_txout_failures(rstate, &scid2));
+	assert(!in_txout_failures(txf, scid1));
+	assert(!in_txout_failures(txf, scid2));
 
-	add_to_txout_failures(rstate, &scid1);
-	assert(in_txout_failures(rstate, &scid1));
-	assert(!in_txout_failures(rstate, &scid2));
-	assert(rstate->num_txout_failures == 1);
+	txout_failures_add(txf, scid1);
+	assert(in_txout_failures(txf, scid1));
+	assert(!in_txout_failures(txf, scid2));
+	assert(txf->num == 1);
 
-	add_to_txout_failures(rstate, &scid2);
-	assert(in_txout_failures(rstate, &scid1));
-	assert(in_txout_failures(rstate, &scid2));
-	assert(rstate->num_txout_failures == 2);
+	txout_failures_add(txf, scid2);
+	assert(in_txout_failures(txf, scid1));
+	assert(in_txout_failures(txf, scid2));
+	assert(txf->num == 2);
 
 	/* Move time forward 1 hour. */
 	t = timers_expire(&daemon->timers,
@@ -136,9 +72,9 @@ int main(int argc, char *argv[])
 	timer_expired(t);
 
 	/* Still there, just old.  Refresh scid1 */
-	assert(rstate->num_txout_failures == 0);
-	assert(in_txout_failures(rstate, &scid1));
-	assert(rstate->num_txout_failures == 1);
+	assert(txf->num == 0);
+	assert(in_txout_failures(txf, scid1));
+	assert(txf->num == 1);
 
 	t = timers_expire(&daemon->timers,
 			  timemono_add(time_mono(),
@@ -146,12 +82,12 @@ int main(int argc, char *argv[])
 	assert(t);
 	timer_expired(t);
 
-	assert(rstate->num_txout_failures == 0);
-	assert(in_txout_failures(rstate, &scid1));
-	assert(rstate->num_txout_failures == 1);
-	assert(!in_txout_failures(rstate, &scid2));
+	assert(txf->num == 0);
+	assert(in_txout_failures(txf, scid1));
+	assert(txf->num == 1);
+	assert(!in_txout_failures(txf, scid2));
 
-	tal_free(rstate);
+	tal_free(txf);
 	timers_cleanup(&daemon->timers);
 	common_shutdown();
 

gossipd/txout_failures.c

@@ -0,0 +1,59 @@
+#include "config.h"
+#include <common/timeout.h>
+#include <gossipd/gossipd.h>
+#include <gossipd/txout_failures.h>
+
+/* Convenience to tell the two failure maps apart */
+#define RECENT 0
+#define AGED 1
+
+#define MAX_ENTRIES 10000
+
+/* Once an hour, or at 10000 entries, we expire old ones */
+static void txout_failures_age(struct txout_failures *txf)
+{
+	uintmap_clear(&txf->failures[AGED]);
+	txf->failures[AGED] = txf->failures[RECENT];
+	uintmap_init(&txf->failures[RECENT]);
+	txf->num = 0;
+
+	txf->age_timer = new_reltimer(&txf->daemon->timers, txf,
+				      time_from_sec(3600),
+				      txout_failures_age, txf);
+}
+
+struct txout_failures *txout_failures_new(const tal_t *ctx, struct daemon *daemon)
+{
+	struct txout_failures *txf = tal(ctx, struct txout_failures);
+
+	txf->daemon = daemon;
+	uintmap_init(&txf->failures[RECENT]);
+	uintmap_init(&txf->failures[AGED]);
+
+	txout_failures_age(txf);
+	return txf;
+}
+
+void txout_failures_add(struct txout_failures *txf,
+			const struct short_channel_id scid)
+{
+	if (uintmap_add(&txf->failures[RECENT], scid.u64, true)
+	    && ++txf->num == MAX_ENTRIES) {
+		tal_free(txf->age_timer);
+		txout_failures_age(txf);
+	}
+}
+
+bool in_txout_failures(struct txout_failures *txf,
+		       const struct short_channel_id scid)
+{
+	if (uintmap_get(&txf->failures[RECENT], scid.u64))
+		return true;
+
+	/* If we were going to expire it, we no longer are. */
+	if (uintmap_get(&txf->failures[AGED], scid.u64)) {
+		txout_failures_add(txf, scid);
+		return true;
+	}
+	return false;
+}

gossipd/txout_failures.h

@@ -0,0 +1,42 @@
+#ifndef LIGHTNING_GOSSIPD_TXOUT_FAILURES_H
+#define LIGHTNING_GOSSIPD_TXOUT_FAILURES_H
+#include "config.h"
+#include <ccan/intmap/intmap.h>
+
+/* Cache for txout queries that failed. Allows us to skip failed
+ * checks if we get another announcement for the same scid. */
+struct txout_failures {
+	/* Cache for txout queries that failed. Allows us to skip failed
+	 * checks if we get another announcement for the same scid. */
+	size_t num;
+	UINTMAP(bool) failures[2];
+	struct oneshot *age_timer;
+
+	/* For access to timers */
+	struct daemon *daemon;
+};
+
+/**
+ * txout_failures_new: allocate a new failure set.
+ * @ctx: tal context
+ * @daemon: global daemon struct
+ */
+struct txout_failures *txout_failures_new(const tal_t *ctx, struct daemon *daemon);
+
+/**
+ * txout_failures_add: add a failed scid to the set.
+ * @txf: the struct txout_failures.
+ * @scid: the short_channel_id to add.
+ */
+void txout_failures_add(struct txout_failures *txf,
+			const struct short_channel_id scid);
+
+/**
+ * in_txout_failures: is this scid in the set?
+ * @txf: the struct txout_failures.
+ * @scid: the short_channel_id to test.
+ */
+bool in_txout_failures(struct txout_failures *txf,
+		       const struct short_channel_id scid);
+
+#endif /* LIGHTNING_GOSSIPD_TXOUT_FAILURES_H */