From 7f45e55d8476e652e19bd2205e17a58ae2760bfb Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 4 Nov 2019 11:07:05 +1030 Subject: [PATCH] gossipd: set the push marker for our own messages. This is a better fix than doing it manually, which turned out to do it in the wrong order (node_announcement followed by channel_announcement) anyway. Should fix many "Bad gossip" messages. Signed-off-by: Rusty Russell --- gossipd/gossip_generation.c | 6 +----- gossipd/gossip_store.c | 13 ++++++++----- gossipd/gossip_store.h | 8 +++++++- gossipd/gossipd.c | 16 ---------------- gossipd/gossipd.h | 3 --- gossipd/routing.c | 13 ++++++++++--- gossipd/test/run-crc32_of_update.c | 3 --- gossipd/test/run-txout_failure.c | 2 +- 8 files changed, 27 insertions(+), 37 deletions(-) diff --git a/gossipd/gossip_generation.c b/gossipd/gossip_generation.c index 657154103..f3ba34800 100644 --- a/gossipd/gossip_generation.c +++ b/gossipd/gossip_generation.c @@ -202,13 +202,12 @@ static void update_own_node_announcement(struct daemon *daemon) /* This injects it into the routing code in routing.c; it should not * reject it! */ - err = handle_node_announcement(daemon->rstate, nannounce, + err = handle_node_announcement(daemon->rstate, take(nannounce), NULL, NULL); if (err) status_failed(STATUS_FAIL_INTERNAL_ERROR, "rejected own node announcement: %s", tal_hex(tmpctx, err)); - push_gossip(daemon, take(nannounce)); } /* Should we announce our own node? Called at strategic places. */ @@ -399,9 +398,6 @@ static void update_local_channel(struct local_cupdate *lc /* frees! */) tal_hex(tmpctx, update), tal_hex(tmpctx, msg)); - if (is_chan_public(chan)) - push_gossip(daemon, take(update)); - tal_free(lc); } diff --git a/gossipd/gossip_store.c b/gossipd/gossip_store.c index d0e6e1d28..eb6306b99 100644 --- a/gossipd/gossip_store.c +++ b/gossipd/gossip_store.c @@ -91,7 +91,8 @@ static ssize_t gossip_pwritev(int fd, const struct iovec *iov, int iovcnt, } #endif /* !HAVE_PWRITEV */ -static bool append_msg(int fd, const u8 *msg, u32 timestamp, u64 *len) +static bool append_msg(int fd, const u8 *msg, u32 timestamp, + bool push, u64 *len) { struct gossip_hdr hdr; u32 msglen; @@ -99,6 +100,8 @@ static bool append_msg(int fd, const u8 *msg, u32 timestamp, u64 *len) msglen = tal_count(msg); hdr.len = cpu_to_be32(msglen); + if (push) + hdr.len |= CPU_TO_BE32(GOSSIP_STORE_LEN_PUSH_BIT); hdr.crc = cpu_to_be32(crc32c(timestamp, msg, msglen)); hdr.timestamp = cpu_to_be32(timestamp); @@ -490,7 +493,7 @@ disable: } u64 gossip_store_add(struct gossip_store *gs, const u8 *gossip_msg, - u32 timestamp, + u32 timestamp, bool push, const u8 *addendum) { u64 off = gs->len; @@ -498,12 +501,12 @@ u64 gossip_store_add(struct gossip_store *gs, const u8 *gossip_msg, /* Should never get here during loading! */ assert(gs->writable); - if (!append_msg(gs->fd, gossip_msg, timestamp, &gs->len)) { + if (!append_msg(gs->fd, gossip_msg, timestamp, push, &gs->len)) { status_broken("Failed writing to gossip store: %s", strerror(errno)); return 0; } - if (addendum && !append_msg(gs->fd, addendum, 0, &gs->len)) { + if (addendum && !append_msg(gs->fd, addendum, 0, false, &gs->len)) { status_broken("Failed writing addendum to gossip store: %s", strerror(errno)); return 0; @@ -520,7 +523,7 @@ u64 gossip_store_add_private_update(struct gossip_store *gs, const u8 *update) /* A local update for an unannounced channel: not broadcastable, but * otherwise the same as a normal channel_update */ const u8 *pupdate = towire_gossip_store_private_update(tmpctx, update); - return gossip_store_add(gs, pupdate, 0, NULL); + return gossip_store_add(gs, pupdate, 0, false, NULL); } /* Returns index of following entry. */ diff --git a/gossipd/gossip_store.h b/gossipd/gossip_store.h index 65c718a51..3695c6017 100644 --- a/gossipd/gossip_store.h +++ b/gossipd/gossip_store.h @@ -35,9 +35,15 @@ u64 gossip_store_add_private_update(struct gossip_store *gs, const u8 *update); /** * Add a gossip message to the gossip_store (and optional addendum) + * @gs: gossip store + * @gossip_msg: the gossip message to insert. + * @timestamp: the timestamp for filtering of this messsage. + * @push: true if this should be sent to peers despite any timestamp filters. + * @addendum: another message to append immediately after this + * (for appending amounts to channel_announcements for internal use). */ u64 gossip_store_add(struct gossip_store *gs, const u8 *gossip_msg, - u32 timestamp, const u8 *addendum); + u32 timestamp, bool push, const u8 *addendum); /** diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index 2aefcb57e..c0040de7d 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -402,21 +402,6 @@ static u8 *handle_node_announce(struct peer *peer, const u8 *msg) return err; } -/*~ Large peers often turn off gossip msgs (using timestamp_filter) from most - * of their peers, however if the gossip is about us, we should spray it to - * everyone whether they've set the filter or not, otherwise it might not - * propagate! */ -void push_gossip(struct daemon *daemon, const u8 *msg TAKES) -{ - struct peer *peer; - - if (taken(msg)) - tal_steal(tmpctx, msg); - - list_for_each(&daemon->peers, peer, list) - queue_peer_msg(peer, msg); -} - static bool handle_local_channel_announcement(struct daemon *daemon, struct peer *peer, const u8 *msg) @@ -441,7 +426,6 @@ static bool handle_local_channel_announcement(struct daemon *daemon, return false; } - push_gossip(daemon, take(cannouncement)); return true; } diff --git a/gossipd/gossipd.h b/gossipd/gossipd.h index fe38d5783..a7321e1cf 100644 --- a/gossipd/gossipd.h +++ b/gossipd/gossipd.h @@ -125,9 +125,6 @@ void peer_supplied_good_gossip(struct peer *peer, size_t amount); struct peer *random_peer(struct daemon *daemon, bool (*check_peer)(const struct peer *peer)); -/* Push this gossip out to all peers immediately. */ -void push_gossip(struct daemon *daemon, const u8 *msg TAKES); - /* Queue a gossip message for the peer: the subdaemon on the other end simply * forwards it to the peer. */ void queue_peer_msg(struct peer *peer, const u8 *msg TAKES); diff --git a/gossipd/routing.c b/gossipd/routing.c index 70fefe0ce..dc89b2a60 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -485,6 +485,7 @@ static void remove_chan_from_node(struct routing_state *rstate, node->bcast.index = gossip_store_add(rstate->gs, announce, node->bcast.timestamp, + false, NULL); } } @@ -1562,6 +1563,7 @@ static void add_channel_announce_to_broadcast(struct routing_state *rstate, u32 index) { u8 *addendum = towire_gossip_store_channel_amount(tmpctx, chan->sat); + bool is_local = is_local_channel(rstate, chan); chan->bcast.timestamp = timestamp; /* 0, unless we're loading from store */ @@ -1571,8 +1573,9 @@ static void add_channel_announce_to_broadcast(struct routing_state *rstate, chan->bcast.index = gossip_store_add(rstate->gs, channel_announce, chan->bcast.timestamp, + is_local, addendum); - rstate->local_channel_announced |= is_local_channel(rstate, chan); + rstate->local_channel_announced |= is_local; } bool routing_add_channel_announcement(struct routing_state *rstate, @@ -2187,6 +2190,7 @@ bool routing_add_channel_update(struct routing_state *rstate, hc->bcast.index = gossip_store_add(rstate->gs, update, hc->bcast.timestamp, + is_local_channel(rstate, chan), NULL); if (hc->bcast.timestamp > rstate->last_timestamp && hc->bcast.timestamp < time_now().ts.tv_sec) @@ -2528,7 +2532,10 @@ bool routing_add_node_announcement(struct routing_state *rstate, else { node->bcast.index = gossip_store_add(rstate->gs, msg, - node->bcast.timestamp, NULL); + node->bcast.timestamp, + node_id_eq(&node_id, + &rstate->local_id), + NULL); peer_supplied_good_gossip(peer, 1); } return true; @@ -2919,7 +2926,7 @@ bool handle_local_add_channel(struct routing_state *rstate, /* Create new (unannounced) channel */ chan = new_chan(rstate, &scid, &rstate->local_id, &remote_node_id, sat); if (!index) - index = gossip_store_add(rstate->gs, msg, 0, NULL); + index = gossip_store_add(rstate->gs, msg, 0, false, NULL); chan->bcast.index = index; return true; } diff --git a/gossipd/test/run-crc32_of_update.c b/gossipd/test/run-crc32_of_update.c index b633f9002..480534a56 100644 --- a/gossipd/test/run-crc32_of_update.c +++ b/gossipd/test/run-crc32_of_update.c @@ -75,9 +75,6 @@ void *notleak_(const void *ptr UNNEEDED, bool plus_children UNNEEDED) /* Generated stub for peer_supplied_good_gossip */ void peer_supplied_good_gossip(struct peer *peer UNNEEDED, size_t amount UNNEEDED) { fprintf(stderr, "peer_supplied_good_gossip called!\n"); abort(); } -/* Generated stub for push_gossip */ -void push_gossip(struct daemon *daemon UNNEEDED, const u8 *msg TAKES UNNEEDED) -{ fprintf(stderr, "push_gossip called!\n"); abort(); } /* Generated stub for queue_peer_from_store */ void queue_peer_from_store(struct peer *peer UNNEEDED, const struct broadcastable *bcast UNNEEDED) diff --git a/gossipd/test/run-txout_failure.c b/gossipd/test/run-txout_failure.c index a082e7f29..4f4564c48 100644 --- a/gossipd/test/run-txout_failure.c +++ b/gossipd/test/run-txout_failure.c @@ -16,7 +16,7 @@ bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct { fprintf(stderr, "fromwire_wireaddr 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, const u8 *addendum UNNEEDED) + u32 timestamp UNNEEDED, bool push UNNEEDED, const u8 *addendum UNNEEDED) { fprintf(stderr, "gossip_store_add called!\n"); abort(); } /* Generated stub for gossip_store_add_private_update */ u64 gossip_store_add_private_update(struct gossip_store *gs UNNEEDED, const u8 *update UNNEEDED)