From a92ead48bf4b015cf314a404dd4add739f528a92 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 16 Sep 2019 20:14:00 +0930 Subject: [PATCH] gossipd: ignore redundant channel_update and node_announcement. If you send a message which simply changes timestamp and signature, we drop it. You shouldn't be doing that, and the door to ignoring them was opened by by option_gossip_query_ex, which would allow clients to ignore updates with the same checksum. This is more aggressive at reducing spam messages, but we allow refreshes (to be conservative, we allow them even when 1/2 of the way through the refresh period). I dropped the now-unnecessary sleep from test_gossip_pruning, too. Signed-off-by: Rusty Russell --- gossipd/routing.c | 67 ++++++++++++++++++-------- gossipd/test/run-bench-find_route.c | 10 ++++ gossipd/test/run-crc32_of_update.c | 43 ++++++++++------- gossipd/test/run-find_route-specific.c | 10 ++++ gossipd/test/run-find_route.c | 10 ++++ gossipd/test/run-overlong.c | 10 ++++ tests/test_gossip.py | 7 --- 7 files changed, 113 insertions(+), 44 deletions(-) diff --git a/gossipd/routing.c b/gossipd/routing.c index 8c5330cbd..d2847e02b 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -1931,17 +1932,31 @@ bool routing_add_channel_update(struct routing_state *rstate, /* Discard older updates */ hc = &chan->half[direction]; - /* If we're loading from store, duplicate entries are a bug. */ - if (is_halfchan_defined(hc) && index != 0) { - status_broken("gossip_store channel_update %u replaces %u!", - index, hc->bcast.index); - return false; - } + if (is_halfchan_defined(hc)) { + /* If we're loading from store, duplicate entries are a bug. */ + if (index != 0) { + status_broken("gossip_store channel_update %u replaces %u!", + index, hc->bcast.index); + return false; + } - if (is_halfchan_defined(hc) && timestamp <= hc->bcast.timestamp) { - SUPERVERBOSE("Ignoring outdated update."); - /* Ignoring != failing */ - return true; + if (timestamp <= hc->bcast.timestamp) { + SUPERVERBOSE("Ignoring outdated update."); + /* Ignoring != failing */ + return true; + } + + /* Allow redundant updates once every 7 days */ + if (timestamp < hc->bcast.timestamp + rstate->prune_timeout / 2 + && !cupdate_different(rstate->gs, hc, update)) { + status_debug("Ignoring redundant update for %s/%u", + type_to_string(tmpctx, + struct short_channel_id, + &short_channel_id), + direction); + /* Ignoring != failing */ + return true; + } } /* FIXME: https://github.com/lightningnetwork/lightning-rfc/pull/512 @@ -2248,15 +2263,29 @@ bool routing_add_node_announcement(struct routing_state *rstate, return true; } - if (node->bcast.index && index != 0) { - status_broken("gossip_store node_announcement %u replaces %u!", - index, node->bcast.index); - return false; - } - if (node->bcast.index && node->bcast.timestamp >= timestamp) { - SUPERVERBOSE("Ignoring node announcement, it's outdated."); - /* OK unless we're loading from store */ - return index == 0; + if (node->bcast.index) { + if (index != 0) { + status_broken("gossip_store node_announcement %u replaces %u!", + index, node->bcast.index); + return false; + } + + if (node->bcast.timestamp >= timestamp) { + SUPERVERBOSE("Ignoring node announcement, it's outdated."); + /* OK unless we're loading from store */ + return index == 0; + } + + /* Allow redundant updates once every 7 days */ + if (timestamp < node->bcast.timestamp + rstate->prune_timeout / 2 + && !nannounce_different(rstate->gs, node, msg)) { + status_debug("Ignoring redundant nannounce for %s", + type_to_string(tmpctx, + struct node_id, + &node_id)); + /* Ignoring != failing */ + return true; + } } /* Harmless if it was never added */ diff --git a/gossipd/test/run-bench-find_route.c b/gossipd/test/run-bench-find_route.c index b72824425..1a288dcb4 100644 --- a/gossipd/test/run-bench-find_route.c +++ b/gossipd/test/run-bench-find_route.c @@ -26,6 +26,11 @@ void status_fmt(enum log_level level UNUSED, const char *fmt, ...) } /* AUTOGENERATED MOCKS START */ +/* Generated stub for cupdate_different */ +bool cupdate_different(struct gossip_store *gs UNNEEDED, + const struct half_chan *hc UNNEEDED, + const u8 *cupdate UNNEEDED) +{ fprintf(stderr, "cupdate_different called!\n"); abort(); } /* Generated stub for fromwire_gossipd_local_add_channel */ bool fromwire_gossipd_local_add_channel(const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct node_id *remote_node_id UNNEEDED, struct amount_sat *satoshis UNNEEDED) { fprintf(stderr, "fromwire_gossipd_local_add_channel called!\n"); abort(); } @@ -41,6 +46,11 @@ bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct /* 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 nannounce_different */ +bool nannounce_different(struct gossip_store *gs UNNEEDED, + const struct node *node UNNEEDED, + const u8 *nannounce UNNEEDED) +{ fprintf(stderr, "nannounce_different called!\n"); abort(); } /* Generated stub for onion_type_name */ const char *onion_type_name(int e UNNEEDED) { fprintf(stderr, "onion_type_name called!\n"); abort(); } diff --git a/gossipd/test/run-crc32_of_update.c b/gossipd/test/run-crc32_of_update.c index 5d87a8e01..600469989 100644 --- a/gossipd/test/run-crc32_of_update.c +++ b/gossipd/test/run-crc32_of_update.c @@ -1,6 +1,7 @@ int unused_main(int argc, char *argv[]); #define main unused_main #include "../gossipd.c" +#include "../make_gossip.c" #undef main #include @@ -68,6 +69,9 @@ bool fromwire_gossip_dev_suppress(const void *p UNNEEDED) /* Generated stub for fromwire_gossipd_get_update */ bool fromwire_gossipd_get_update(const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED) { fprintf(stderr, "fromwire_gossipd_get_update called!\n"); abort(); } +/* Generated stub for fromwire_gossipd_local_channel_update */ +bool fromwire_gossipd_local_channel_update(const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, bool *disable UNNEEDED, u16 *cltv_expiry_delta UNNEEDED, struct amount_msat *htlc_minimum_msat UNNEEDED, u32 *fee_base_msat UNNEEDED, u32 *fee_proportional_millionths UNNEEDED, struct amount_msat *htlc_maximum_msat UNNEEDED) +{ fprintf(stderr, "fromwire_gossipd_local_channel_update called!\n"); abort(); } /* Generated stub for fromwire_gossip_get_addrs */ bool fromwire_gossip_get_addrs(const void *p UNNEEDED, struct node_id *id UNNEEDED) { fprintf(stderr, "fromwire_gossip_get_addrs called!\n"); abort(); } @@ -113,17 +117,18 @@ bool fromwire_gossip_query_scids(const tal_t *ctx UNNEEDED, const void *p UNNEED /* Generated stub for fromwire_gossip_send_timestamp_filter */ bool fromwire_gossip_send_timestamp_filter(const void *p UNNEEDED, struct node_id *id UNNEEDED, u32 *first_timestamp UNNEEDED, u32 *timestamp_range UNNEEDED) { fprintf(stderr, "fromwire_gossip_send_timestamp_filter called!\n"); abort(); } +/* Generated stub for fromwire_hsm_cupdate_sig_reply */ +bool fromwire_hsm_cupdate_sig_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u8 **cu UNNEEDED) +{ fprintf(stderr, "fromwire_hsm_cupdate_sig_reply called!\n"); abort(); } +/* Generated stub for fromwire_hsm_node_announcement_sig_reply */ +bool fromwire_hsm_node_announcement_sig_reply(const void *p UNNEEDED, secp256k1_ecdsa_signature *signature UNNEEDED) +{ fprintf(stderr, "fromwire_hsm_node_announcement_sig_reply called!\n"); abort(); } /* Generated stub for fromwire_incorrect_cltv_expiry */ bool fromwire_incorrect_cltv_expiry(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u32 *cltv_expiry UNNEEDED, u8 **channel_update UNNEEDED) { fprintf(stderr, "fromwire_incorrect_cltv_expiry called!\n"); abort(); } /* Generated stub for fromwire_temporary_channel_failure */ bool fromwire_temporary_channel_failure(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u8 **channel_update UNNEEDED) { fprintf(stderr, "fromwire_temporary_channel_failure 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 get_node */ struct node *get_node(struct routing_state *rstate UNNEEDED, const struct node_id *id UNNEEDED) @@ -176,11 +181,6 @@ u8 *handle_channel_update(struct routing_state *rstate UNNEEDED, const u8 *updat bool handle_local_add_channel(struct routing_state *rstate UNNEEDED, const u8 *msg UNNEEDED, u64 index UNNEEDED) { fprintf(stderr, "handle_local_add_channel called!\n"); abort(); } -/* Generated stub for handle_local_channel_update */ -bool handle_local_channel_update(struct daemon *daemon UNNEEDED, - const struct node_id *src UNNEEDED, - const u8 *msg UNNEEDED) -{ fprintf(stderr, "handle_local_channel_update called!\n"); abort(); } /* Generated stub for handle_node_announcement */ u8 *handle_node_announcement(struct routing_state *rstate UNNEEDED, const u8 *node UNNEEDED) { fprintf(stderr, "handle_node_announcement called!\n"); abort(); } @@ -196,9 +196,6 @@ u8 *make_ping(const tal_t *ctx UNNEEDED, u16 num_pong_bytes UNNEEDED, u16 padlen /* Generated stub for master_badmsg */ void master_badmsg(u32 type_expected UNNEEDED, const u8 *msg) { fprintf(stderr, "master_badmsg called!\n"); abort(); } -/* Generated stub for maybe_send_own_node_announce */ -void maybe_send_own_node_announce(struct daemon *daemon UNNEEDED) -{ fprintf(stderr, "maybe_send_own_node_announce called!\n"); abort(); } /* Generated stub for memleak_enter_allocations */ struct htable *memleak_enter_allocations(const tal_t *ctx UNNEEDED, const void *exclude1 UNNEEDED, @@ -230,11 +227,6 @@ void *notleak_(const void *ptr UNNEEDED, bool plus_children UNNEEDED) /* Generated stub for read_addresses */ struct wireaddr *read_addresses(const tal_t *ctx UNNEEDED, const u8 *ser UNNEEDED) { fprintf(stderr, "read_addresses called!\n"); abort(); } -/* Generated stub for refresh_local_channel */ -void refresh_local_channel(struct daemon *daemon UNNEEDED, - struct local_chan *local_chan UNNEEDED, - bool even_if_identical UNNEEDED) -{ fprintf(stderr, "refresh_local_channel called!\n"); abort(); } /* Generated stub for remove_channel_from_store */ void remove_channel_from_store(struct routing_state *rstate UNNEEDED, struct chan *chan UNNEEDED) @@ -317,6 +309,21 @@ u8 *towire_gossip_query_channel_range_reply(const tal_t *ctx UNNEEDED, u32 final /* Generated stub for towire_gossip_scids_reply */ u8 *towire_gossip_scids_reply(const tal_t *ctx UNNEEDED, bool ok UNNEEDED, bool complete UNNEEDED) { fprintf(stderr, "towire_gossip_scids_reply called!\n"); abort(); } +/* Generated stub for towire_hsm_cupdate_sig_req */ +u8 *towire_hsm_cupdate_sig_req(const tal_t *ctx UNNEEDED, const u8 *cu UNNEEDED) +{ fprintf(stderr, "towire_hsm_cupdate_sig_req called!\n"); abort(); } +/* Generated stub for towire_hsm_node_announcement_sig_req */ +u8 *towire_hsm_node_announcement_sig_req(const tal_t *ctx UNNEEDED, const u8 *announcement UNNEEDED) +{ fprintf(stderr, "towire_hsm_node_announcement_sig_req called!\n"); abort(); } +/* Generated stub for towire_wireaddr */ +void towire_wireaddr(u8 **pptr UNNEEDED, const struct wireaddr *addr UNNEEDED) +{ fprintf(stderr, "towire_wireaddr called!\n"); abort(); } +/* Generated stub for wire_sync_read */ +u8 *wire_sync_read(const tal_t *ctx UNNEEDED, int fd UNNEEDED) +{ fprintf(stderr, "wire_sync_read called!\n"); abort(); } +/* Generated stub for wire_sync_write */ +bool wire_sync_write(int fd UNNEEDED, const void *msg TAKES UNNEEDED) +{ fprintf(stderr, "wire_sync_write called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ #if EXPERIMENTAL_FEATURES diff --git a/gossipd/test/run-find_route-specific.c b/gossipd/test/run-find_route-specific.c index 0f2a23379..92e09c963 100644 --- a/gossipd/test/run-find_route-specific.c +++ b/gossipd/test/run-find_route-specific.c @@ -15,6 +15,11 @@ #include "../gossip_store.c" /* AUTOGENERATED MOCKS START */ +/* Generated stub for cupdate_different */ +bool cupdate_different(struct gossip_store *gs UNNEEDED, + const struct half_chan *hc UNNEEDED, + const u8 *cupdate UNNEEDED) +{ fprintf(stderr, "cupdate_different called!\n"); abort(); } /* Generated stub for fromwire_gossipd_local_add_channel */ bool fromwire_gossipd_local_add_channel(const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct node_id *remote_node_id UNNEEDED, struct amount_sat *satoshis UNNEEDED) { fprintf(stderr, "fromwire_gossipd_local_add_channel called!\n"); abort(); } @@ -30,6 +35,11 @@ bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct /* 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 nannounce_different */ +bool nannounce_different(struct gossip_store *gs UNNEEDED, + const struct node *node UNNEEDED, + const u8 *nannounce UNNEEDED) +{ fprintf(stderr, "nannounce_different called!\n"); abort(); } /* Generated stub for onion_type_name */ const char *onion_type_name(int e UNNEEDED) { fprintf(stderr, "onion_type_name called!\n"); abort(); } diff --git a/gossipd/test/run-find_route.c b/gossipd/test/run-find_route.c index d8ae06b64..ea9e7346a 100644 --- a/gossipd/test/run-find_route.c +++ b/gossipd/test/run-find_route.c @@ -13,6 +13,11 @@ void status_fmt(enum log_level level UNUSED, const char *fmt, ...) } /* AUTOGENERATED MOCKS START */ +/* Generated stub for cupdate_different */ +bool cupdate_different(struct gossip_store *gs UNNEEDED, + const struct half_chan *hc UNNEEDED, + const u8 *cupdate UNNEEDED) +{ fprintf(stderr, "cupdate_different called!\n"); abort(); } /* Generated stub for fromwire_gossipd_local_add_channel */ bool fromwire_gossipd_local_add_channel(const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct node_id *remote_node_id UNNEEDED, struct amount_sat *satoshis UNNEEDED) { fprintf(stderr, "fromwire_gossipd_local_add_channel called!\n"); abort(); } @@ -28,6 +33,11 @@ bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct /* 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 nannounce_different */ +bool nannounce_different(struct gossip_store *gs UNNEEDED, + const struct node *node UNNEEDED, + const u8 *nannounce UNNEEDED) +{ fprintf(stderr, "nannounce_different called!\n"); abort(); } /* Generated stub for onion_type_name */ const char *onion_type_name(int e UNNEEDED) { fprintf(stderr, "onion_type_name called!\n"); abort(); } diff --git a/gossipd/test/run-overlong.c b/gossipd/test/run-overlong.c index 357046b4b..324af4c9c 100644 --- a/gossipd/test/run-overlong.c +++ b/gossipd/test/run-overlong.c @@ -13,6 +13,11 @@ void status_fmt(enum log_level level UNUSED, const char *fmt, ...) } /* AUTOGENERATED MOCKS START */ +/* Generated stub for cupdate_different */ +bool cupdate_different(struct gossip_store *gs UNNEEDED, + const struct half_chan *hc UNNEEDED, + const u8 *cupdate UNNEEDED) +{ fprintf(stderr, "cupdate_different called!\n"); abort(); } /* Generated stub for fromwire_gossipd_local_add_channel */ bool fromwire_gossipd_local_add_channel(const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct node_id *remote_node_id UNNEEDED, struct amount_sat *satoshis UNNEEDED) { fprintf(stderr, "fromwire_gossipd_local_add_channel called!\n"); abort(); } @@ -28,6 +33,11 @@ bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct /* 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 nannounce_different */ +bool nannounce_different(struct gossip_store *gs UNNEEDED, + const struct node *node UNNEEDED, + const u8 *nannounce UNNEEDED) +{ fprintf(stderr, "nannounce_different called!\n"); abort(); } /* Generated stub for onion_type_name */ const char *onion_type_name(int e UNNEEDED) { fprintf(stderr, "onion_type_name called!\n"); abort(); } diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 7726c8535..f0d8496c5 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -51,13 +51,6 @@ def test_gossip_pruning(node_factory, bitcoind): ]) # Now kill l3, so that l2 and l1 can prune it from their view after 10 seconds - - # FIXME: This sleep() masks a real bug: that channeld sends a - # channel_update message (to disable the channel) with same - # timestamp as the last keepalive, and thus is ignored. The minimal - # fix is to backdate the keepalives 1 second, but maybe we should - # simply have gossipd generate all updates? - time.sleep(1) l3.stop() l1.daemon.wait_for_log("Pruning channel {} from network view".format(scid2))