From 652432a298b9905c7ce4c585b29df12424803974 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 31 Jan 2024 14:52:33 +1030 Subject: [PATCH] gossipd: take txout failure cache out of routing.c Signed-off-by: Rusty Russell --- gossipd/Makefile | 1 + gossipd/routing.c | 51 +------- gossipd/routing.h | 4 +- gossipd/test/run-check_channel_announcement.c | 17 ++- gossipd/test/run-txout_failure.c | 112 ++++-------------- gossipd/txout_failures.c | 59 +++++++++ gossipd/txout_failures.h | 42 +++++++ 7 files changed, 144 insertions(+), 142 deletions(-) create mode 100644 gossipd/txout_failures.c create mode 100644 gossipd/txout_failures.h diff --git a/gossipd/Makefile b/gossipd/Makefile index 42e79f256..061d08167 100644 --- a/gossipd/Makefile +++ b/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 diff --git a/gossipd/routing.c b/gossipd/routing.c index 65f620b0f..23c093d6d 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -16,6 +16,7 @@ #include #include #include +#include #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 diff --git a/gossipd/routing.h b/gossipd/routing.h index 71062b658..3f8946e59 100644 --- a/gossipd/routing.h +++ b/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; diff --git a/gossipd/test/run-check_channel_announcement.c b/gossipd/test/run-check_channel_announcement.c index 95eb5a2a0..77184b16b 100644 --- a/gossipd/test/run-check_channel_announcement.c +++ b/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[]) diff --git a/gossipd/test/run-txout_failure.c b/gossipd/test/run-txout_failure.c index d9047c994..70b079ff9 100644 --- a/gossipd/test/run-txout_failure.c +++ b/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 #include +#include #include #include #include #include +#include +#include #include /* 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(); diff --git a/gossipd/txout_failures.c b/gossipd/txout_failures.c new file mode 100644 index 000000000..823f150e1 --- /dev/null +++ b/gossipd/txout_failures.c @@ -0,0 +1,59 @@ +#include "config.h" +#include +#include +#include + +/* 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; +} diff --git a/gossipd/txout_failures.h b/gossipd/txout_failures.h new file mode 100644 index 000000000..6afcbc3bd --- /dev/null +++ b/gossipd/txout_failures.h @@ -0,0 +1,42 @@ +#ifndef LIGHTNING_GOSSIPD_TXOUT_FAILURES_H +#define LIGHTNING_GOSSIPD_TXOUT_FAILURES_H +#include "config.h" +#include + +/* 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 */