From 0e0ad1aa4dbd20c267e7e25e9222ff20719520ef Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Thu, 29 Mar 2018 13:29:01 +0200 Subject: [PATCH] gossip: Check that we have a node before applying changes This was a tricky one to find, it turns out that some nodes are sending node_announcements even if they don't have a channel announced yet. If they are a peer and the channel is currently verifying then we'll have a local channel in the network view, hence accept the node_announcement, but when replaying, the node_announcement will be replayed and we won't have a channel yet. This just skips node_announcements, which is always safe. Reported-by: @laszlohanyecz Signed-off-by: Christian Decker --- gossipd/routing.c | 13 +++++++++++-- gossipd/routing.h | 2 +- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/gossipd/routing.c b/gossipd/routing.c index 004edcaa6..6728d8fb3 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -1062,7 +1062,7 @@ static struct wireaddr *read_addresses(const tal_t *ctx, const u8 *ser) return wireaddrs; } -void routing_add_node_announcement(struct routing_state *rstate, const u8 *msg TAKES) +bool routing_add_node_announcement(struct routing_state *rstate, const u8 *msg TAKES) { struct node *node; secp256k1_ecdsa_signature signature; @@ -1078,6 +1078,12 @@ void routing_add_node_announcement(struct routing_state *rstate, const u8 *msg T &addresses); node = get_node(rstate, &node_id); + + /* May happen if we accepted the node_announcement due to a local + * channel, for which we didn't have the announcement hust yet. */ + if (node == NULL) + return false; + wireaddrs = read_addresses(tmpctx, addresses); tal_free(node->addresses); node->addresses = tal_steal(node, wireaddrs); @@ -1090,6 +1096,7 @@ void routing_add_node_announcement(struct routing_state *rstate, const u8 *msg T replace_broadcast(node, rstate->broadcasts, &node->node_announce_msgidx, take(msg)); + return true; } u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann) @@ -1106,6 +1113,7 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann) struct wireaddr *wireaddrs; struct pending_node_announce *pna; size_t len = tal_len(node_ann); + bool applied; serialized = tal_dup_arr(tmpctx, u8, node_ann, len, 0); if (!fromwire_node_announcement(tmpctx, serialized, @@ -1217,7 +1225,8 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann) type_to_string(tmpctx, struct pubkey, &node_id)); gossip_store_add_node_announcement(rstate->store, serialized); - routing_add_node_announcement(rstate, serialized); + applied = routing_add_node_announcement(rstate, serialized); + assert(applied); return NULL; } diff --git a/gossipd/routing.h b/gossipd/routing.h index 32ca3c7a8..16d05ea5f 100644 --- a/gossipd/routing.h +++ b/gossipd/routing.h @@ -286,7 +286,7 @@ void routing_add_channel_update(struct routing_state *rstate, * it. This must be from a trusted source, e.g., gossip_store. For untrusted * sources (peers) please use @see{handle_node_announcement}. */ -void routing_add_node_announcement(struct routing_state *rstate, +bool routing_add_node_announcement(struct routing_state *rstate, const u8 *msg TAKES); #endif /* LIGHTNING_GOSSIPD_ROUTING_H */