From 0091300ee3e9bf286595abe2b0140b0afd54151a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 8 Oct 2019 11:43:24 +1030 Subject: [PATCH] gossipd: track what peer gave us gossip msgs so we can credit it. Since we have to validate, there can be a delay (and peer might vanish) between receiving the gossip and actually confirming it, hence the use of softref. We will use this information to check that the peers are making progress as we start asking them for specific information. Signed-off-by: Rusty Russell --- gossipd/gossip_generation.c | 4 +- gossipd/gossip_store.c | 9 ++-- gossipd/gossipd.c | 17 ++++-- gossipd/gossipd.h | 6 +++ gossipd/routing.c | 71 ++++++++++++++++++-------- gossipd/routing.h | 29 ++++++++--- gossipd/test/run-bench-find_route.c | 3 ++ gossipd/test/run-crc32_of_update.c | 4 +- gossipd/test/run-find_route-specific.c | 3 ++ gossipd/test/run-find_route.c | 3 ++ gossipd/test/run-overlong.c | 3 ++ gossipd/test/run-txout_failure.c | 3 ++ 12 files changed, 118 insertions(+), 37 deletions(-) diff --git a/gossipd/gossip_generation.c b/gossipd/gossip_generation.c index baf048825..0e906740f 100644 --- a/gossipd/gossip_generation.c +++ b/gossipd/gossip_generation.c @@ -202,7 +202,7 @@ 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, take(nannounce)); + err = handle_node_announcement(daemon->rstate, take(nannounce), NULL); if (err) status_failed(STATUS_FAIL_INTERNAL_ERROR, "rejected own node announcement: %s", @@ -383,7 +383,7 @@ static void update_local_channel(struct local_cupdate *lc /* frees! */) * about it being invalid! __func__ is a magic C constant which * expands to this function name. */ msg = handle_channel_update(daemon->rstate, take(update), __func__, - NULL); + NULL, NULL); if (msg) status_failed(STATUS_FAIL_INTERNAL_ERROR, "%s: rejected local channel update %s: %s", diff --git a/gossipd/gossip_store.c b/gossipd/gossip_store.c index cb7f4589c..30c796069 100644 --- a/gossipd/gossip_store.c +++ b/gossipd/gossip_store.c @@ -686,7 +686,8 @@ u32 gossip_store_load(struct routing_state *rstate, struct gossip_store *gs) if (!routing_add_channel_announcement(rstate, take(chan_ann), satoshis, - chan_ann_off)) { + chan_ann_off, + NULL)) { bad = "Bad channel_announcement"; goto badmsg; } @@ -710,7 +711,8 @@ u32 gossip_store_load(struct routing_state *rstate, struct gossip_store *gs) /* fall thru */ case WIRE_CHANNEL_UPDATE: if (!routing_add_channel_update(rstate, - take(msg), gs->len)) { + take(msg), gs->len, + NULL)) { bad = "Bad channel_update"; goto badmsg; } @@ -718,7 +720,8 @@ u32 gossip_store_load(struct routing_state *rstate, struct gossip_store *gs) break; case WIRE_NODE_ANNOUNCEMENT: if (!routing_add_node_announcement(rstate, - take(msg), gs->len)) { + take(msg), gs->len, + NULL)) { bad = "Bad node_announcement"; goto badmsg; } diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index 578501197..a7dcf56ac 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -133,6 +133,13 @@ struct peer *find_peer(struct daemon *daemon, const struct node_id *id) return NULL; } +/* Increment a peer's gossip_counter, if peer not NULL */ +void peer_supplied_good_gossip(struct peer *peer) +{ + if (peer) + peer->gossip_counter++; +} + /* 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) @@ -290,7 +297,7 @@ static const u8 *handle_channel_announcement_msg(struct peer *peer, * which case, it frees and NULLs that ptr) */ err = handle_channel_announcement(peer->daemon->rstate, msg, peer->daemon->current_blockheight, - &scid); + &scid, peer); if (err) return err; else if (scid) { @@ -317,7 +324,7 @@ static u8 *handle_channel_update_msg(struct peer *peer, const u8 *msg) unknown_scid.u64 = 0; err = handle_channel_update(peer->daemon->rstate, msg, "subdaemon", - &unknown_scid); + peer, &unknown_scid); if (err) { if (unknown_scid.u64 != 0) query_unknown_channel(peer->daemon, peer, &unknown_scid); @@ -462,7 +469,7 @@ static struct io_plan *peer_msg_in(struct io_conn *conn, err = handle_channel_update_msg(peer, msg); goto handled_relay; case WIRE_NODE_ANNOUNCEMENT: - err = handle_node_announcement(peer->daemon->rstate, msg); + err = handle_node_announcement(peer->daemon->rstate, msg, peer); goto handled_relay; case WIRE_QUERY_CHANNEL_RANGE: err = handle_query_channel_range(peer, msg); @@ -631,6 +638,7 @@ static struct io_plan *connectd_new_peer(struct io_conn *conn, /* Populate the rest of the peer info. */ peer->daemon = daemon; + peer->gossip_counter = 0; peer->scid_queries = NULL; peer->scid_query_idx = 0; peer->scid_query_nodes = NULL; @@ -1525,7 +1533,8 @@ static struct io_plan *handle_txout_reply(struct io_conn *conn, } /* Outscript is NULL if it's not an unspent output */ - if (handle_pending_cannouncement(daemon->rstate, &scid, sat, outscript) + if (handle_pending_cannouncement(daemon, daemon->rstate, + &scid, sat, outscript) && was_unknown) { /* It was real: we're missing gossip. */ gossip_missing(daemon); diff --git a/gossipd/gossipd.h b/gossipd/gossipd.h index 9208201b6..92ab5af6d 100644 --- a/gossipd/gossipd.h +++ b/gossipd/gossipd.h @@ -89,6 +89,9 @@ struct peer { /* The ID of the peer (always unique) */ struct node_id id; + /* How much contribution have we made to gossip? */ + size_t gossip_counter; + /* The two features gossip cares about (so far) */ bool gossip_queries_feature, initial_routing_sync_feature; @@ -129,6 +132,9 @@ struct peer { /* Search for a peer. */ struct peer *find_peer(struct daemon *daemon, const struct node_id *id); +/* This peer (may be NULL) gave is valid gossip. */ +void peer_supplied_good_gossip(struct peer *peer); + /* 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 cce6615d2..f87bb63fd 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -35,6 +36,8 @@ struct pending_node_announce { u8 *node_announcement; u32 timestamp; u32 index; + /* Automagically turns to NULL if peer freed */ + struct peer *peer_softref; }; /* We consider a reasonable gossip rate to be 1 per day, with burst of @@ -95,6 +98,8 @@ struct unupdated_channel { u32 index; /* Channel capacity */ struct amount_sat sat; + /* Automagically turns to NULL of peer freed */ + struct peer *peer_softref; }; static struct unupdated_channel * @@ -1481,6 +1486,7 @@ static void catch_node_announcement(const tal_t *ctx, pna->timestamp = 0; pna->index = 0; pna->refcount = 0; + pna->peer_softref = NULL; pending_node_map_add(rstate->pending_node_map, pna); } pna->refcount++; @@ -1502,7 +1508,8 @@ static void process_pending_node_announcement(struct routing_state *rstate, /* Can fail it timestamp is now too old */ if (!routing_add_node_announcement(rstate, pna->node_announcement, - pna->index)) + pna->index, + pna->peer_softref)) status_unusual("pending node_announcement %s too old?", tal_hex(tmpctx, pna->node_announcement)); /* Never send this again. */ @@ -1563,7 +1570,8 @@ static void add_channel_announce_to_broadcast(struct routing_state *rstate, bool routing_add_channel_announcement(struct routing_state *rstate, const u8 *msg TAKES, struct amount_sat sat, - u32 index) + u32 index, + struct peer *peer) { struct chan *chan; secp256k1_ecdsa_signature node_signature_1, node_signature_2; @@ -1629,6 +1637,7 @@ bool routing_add_channel_announcement(struct routing_state *rstate, uc->scid = scid; uc->id[0] = node_id_1; uc->id[1] = node_id_2; + set_softref(uc, &uc->peer_softref, peer); uintmap_add(&rstate->unupdated_chanmap, scid.u64, uc); tal_add_destructor2(uc, destroy_unupdated_channel, rstate); @@ -1638,9 +1647,11 @@ bool routing_add_channel_announcement(struct routing_state *rstate, /* If we had private updates, they'll immediately create the channel. */ if (private_updates[0]) - routing_add_channel_update(rstate, take(private_updates[0]), 0); + routing_add_channel_update(rstate, take(private_updates[0]), 0, + peer); if (private_updates[1]) - routing_add_channel_update(rstate, take(private_updates[1]), 0); + routing_add_channel_update(rstate, take(private_updates[1]), 0, + peer); return true; } @@ -1648,7 +1659,8 @@ bool routing_add_channel_announcement(struct routing_state *rstate, u8 *handle_channel_announcement(struct routing_state *rstate, const u8 *announce TAKES, u32 current_blockheight, - const struct short_channel_id **scid) + const struct short_channel_id **scid, + struct peer *peer) { struct pending_cannouncement *pending; struct bitcoin_blkid chain_hash; @@ -1658,8 +1670,10 @@ u8 *handle_channel_announcement(struct routing_state *rstate, struct chan *chan; pending = tal(rstate, struct pending_cannouncement); + set_softref(pending, &pending->peer_softref, peer); pending->updates[0] = NULL; pending->updates[1] = NULL; + pending->update_peer_softref[0] = pending->update_peer_softref[1] = NULL; pending->announce = tal_dup_arr(pending, u8, announce, tal_count(announce), 0); pending->update_timestamps[0] = pending->update_timestamps[1] = 0; @@ -1834,9 +1848,11 @@ ignored: return NULL; } -static void process_pending_channel_update(struct routing_state *rstate, +static void process_pending_channel_update(struct daemon *daemon, + struct routing_state *rstate, const struct short_channel_id *scid, - const u8 *cupdate) + const u8 *cupdate, + struct peer *peer) { u8 *err; @@ -1844,7 +1860,8 @@ static void process_pending_channel_update(struct routing_state *rstate, return; /* FIXME: We don't remember who sent us updates, so can't error them */ - err = handle_channel_update(rstate, cupdate, "pending update", NULL); + err = handle_channel_update(rstate, cupdate, "pending update", peer, + NULL); if (err) { status_debug("Pending channel_update for %s: %s", type_to_string(tmpctx, struct short_channel_id, scid), @@ -1853,7 +1870,8 @@ static void process_pending_channel_update(struct routing_state *rstate, } } -bool handle_pending_cannouncement(struct routing_state *rstate, +bool handle_pending_cannouncement(struct daemon *daemon, + struct routing_state *rstate, const struct short_channel_id *scid, struct amount_sat sat, const u8 *outscript) @@ -1909,13 +1927,16 @@ bool handle_pending_cannouncement(struct routing_state *rstate, tal_del_destructor2(pending, destroy_pending_cannouncement, rstate); /* Can fail if channel_announcement too old */ - if (!routing_add_channel_announcement(rstate, pending->announce, sat, 0)) + if (!routing_add_channel_announcement(rstate, pending->announce, sat, 0, + pending->peer_softref)) status_unusual("Could not add channel_announcement %s: too old?", tal_hex(tmpctx, pending->announce)); else { /* Did we have an update waiting? If so, apply now. */ - process_pending_channel_update(rstate, scid, pending->updates[0]); - process_pending_channel_update(rstate, scid, pending->updates[1]); + process_pending_channel_update(daemon, rstate, scid, pending->updates[0], + pending->update_peer_softref[0]); + process_pending_channel_update(daemon, rstate, scid, pending->updates[1], + pending->update_peer_softref[1]); } tal_free(pending); @@ -1970,7 +1991,8 @@ static void set_connection_values(struct chan *chan, bool routing_add_channel_update(struct routing_state *rstate, const u8 *update TAKES, - u32 index) + u32 index, + struct peer *peer) { secp256k1_ecdsa_signature signature; struct short_channel_id short_channel_id; @@ -2147,11 +2169,13 @@ bool routing_add_channel_update(struct routing_state *rstate, /* If we're loading from store, this means we don't re-add to store. */ if (index) hc->bcast.index = index; - else + else { hc->bcast.index = gossip_store_add(rstate->gs, update, hc->bcast.timestamp, NULL); + peer_supplied_good_gossip(peer); + } if (uc) { /* If we were waiting for these nodes to appear (or gain a @@ -2204,6 +2228,7 @@ void remove_channel_from_store(struct routing_state *rstate, u8 *handle_channel_update(struct routing_state *rstate, const u8 *update TAKES, const char *source, + struct peer *peer, struct short_channel_id *unknown_scid) { u8 *serialized; @@ -2305,7 +2330,7 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update TAKES, channel_flags & ROUTING_FLAGS_DISABLED ? "DISABLED" : "ACTIVE", source); - routing_add_channel_update(rstate, take(serialized), 0); + routing_add_channel_update(rstate, take(serialized), 0, peer); return NULL; } @@ -2342,7 +2367,8 @@ struct wireaddr *read_addresses(const tal_t *ctx, const u8 *ser) bool routing_add_node_announcement(struct routing_state *rstate, const u8 *msg TAKES, - u32 index) + u32 index, + struct peer *peer) { struct node *node; secp256k1_ecdsa_signature signature; @@ -2407,9 +2433,11 @@ bool routing_add_node_announcement(struct routing_state *rstate, pna->timestamp = timestamp; pna->index = index; tal_free(pna->node_announcement); + clear_softref(pna, &pna->peer_softref); pna->node_announcement = tal_dup_arr(pna, u8, msg, tal_count(msg), 0); + set_softref(pna, &pna->peer_softref, peer); return true; } @@ -2461,14 +2489,17 @@ bool routing_add_node_announcement(struct routing_state *rstate, node->bcast.timestamp = timestamp; if (index) node->bcast.index = index; - else + else { node->bcast.index = gossip_store_add(rstate->gs, msg, node->bcast.timestamp, NULL); + peer_supplied_good_gossip(peer); + } return true; } -u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann) +u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann, + struct peer *peer) { u8 *serialized; struct sha256_double hash; @@ -2556,7 +2587,7 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann) } /* May still fail, if we don't know the node. */ - routing_add_node_announcement(rstate, serialized, 0); + routing_add_node_announcement(rstate, serialized, 0, peer); return NULL; } @@ -2701,7 +2732,7 @@ void routing_failure(struct routing_state *rstate, /* lightningd will only extract this if UPDATE is set. */ if (channel_update) { u8 *err = handle_channel_update(rstate, channel_update, "error", - NULL); + NULL, NULL); if (err) { status_unusual("routing_failure: " "bad channel_update %s", diff --git a/gossipd/routing.h b/gossipd/routing.h index f7bc88f49..c9db41232 100644 --- a/gossipd/routing.h +++ b/gossipd/routing.h @@ -14,6 +14,8 @@ #include #include +struct daemon; +struct peer; struct routing_state; struct half_chan { @@ -177,12 +179,17 @@ struct pending_cannouncement { struct pubkey bitcoin_key_1; struct pubkey bitcoin_key_2; + /* Automagically turns to NULL of peer freed */ + struct peer *peer_softref; + /* The raw bits */ const u8 *announce; /* Deferred updates, if we received them while waiting for * this (one for each direction) */ const u8 *updates[2]; + /* Peers responsible: turns to NULL if they're freed */ + struct peer *update_peer_softref[2]; /* Only ever replace with newer updates */ u32 update_timestamps[2]; @@ -357,14 +364,16 @@ struct chan *new_chan(struct routing_state *rstate, u8 *handle_channel_announcement(struct routing_state *rstate, const u8 *announce TAKES, u32 current_blockheight, - const struct short_channel_id **scid); + const struct short_channel_id **scid, + struct peer *peer); /** * handle_pending_cannouncement -- handle channel_announce once we've * completed short_channel_id lookup. Returns true if handling created * a new channel. */ -bool handle_pending_cannouncement(struct routing_state *rstate, +bool handle_pending_cannouncement(struct daemon *daemon, + struct routing_state *rstate, const struct short_channel_id *scid, const struct amount_sat sat, const u8 *txscript); @@ -378,10 +387,12 @@ struct chan *next_chan(const struct node *node, struct chan_map_iter *i); * (if not NULL). */ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update TAKES, const char *source, + struct peer *peer, struct short_channel_id *unknown_scid); /* Returns NULL if all OK, otherwise an error for the peer which sent. */ -u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node); +u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node, + struct peer *peer); /* Get a node: use this instead of node_map_get() */ struct node *get_node(struct routing_state *rstate, @@ -416,11 +427,14 @@ void route_prune(struct routing_state *rstate); * * index is usually 0, in which case it's set by insert_broadcast adding it * to the store. + * + * peer is an optional peer responsible for this. */ bool routing_add_channel_announcement(struct routing_state *rstate, const u8 *msg TAKES, struct amount_sat sat, - u32 index); + u32 index, + struct peer *peer); /** * Add a channel_update without checking for errors @@ -432,8 +446,8 @@ bool routing_add_channel_announcement(struct routing_state *rstate, */ bool routing_add_channel_update(struct routing_state *rstate, const u8 *update TAKES, - u32 index); - + u32 index, + struct peer *peer); /** * Add a node_announcement to the network view without checking it * @@ -443,7 +457,8 @@ bool routing_add_channel_update(struct routing_state *rstate, */ bool routing_add_node_announcement(struct routing_state *rstate, const u8 *msg TAKES, - u32 index); + u32 index, + struct peer *peer); /** diff --git a/gossipd/test/run-bench-find_route.c b/gossipd/test/run-bench-find_route.c index 958ded7a6..8f072fc40 100644 --- a/gossipd/test/run-bench-find_route.c +++ b/gossipd/test/run-bench-find_route.c @@ -54,6 +54,9 @@ bool nannounce_different(struct gossip_store *gs UNNEEDED, /* Generated stub for onion_type_name */ const char *onion_type_name(int e UNNEEDED) { fprintf(stderr, "onion_type_name called!\n"); abort(); } +/* Generated stub for peer_supplied_good_gossip */ +void peer_supplied_good_gossip(struct peer *peer 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) diff --git a/gossipd/test/run-crc32_of_update.c b/gossipd/test/run-crc32_of_update.c index 7dda1436b..c4a5a31aa 100644 --- a/gossipd/test/run-crc32_of_update.c +++ b/gossipd/test/run-crc32_of_update.c @@ -50,10 +50,12 @@ struct timeabs gossip_time_now(const struct routing_state *rstate UNNEEDED) /* Generated stub for handle_channel_update */ u8 *handle_channel_update(struct routing_state *rstate UNNEEDED, const u8 *update TAKES UNNEEDED, const char *source UNNEEDED, + struct peer *peer UNNEEDED, struct short_channel_id *unknown_scid UNNEEDED) { fprintf(stderr, "handle_channel_update called!\n"); abort(); } /* Generated stub for handle_node_announcement */ -u8 *handle_node_announcement(struct routing_state *rstate UNNEEDED, const u8 *node UNNEEDED) +u8 *handle_node_announcement(struct routing_state *rstate UNNEEDED, const u8 *node UNNEEDED, + struct peer *peer UNNEEDED) { fprintf(stderr, "handle_node_announcement called!\n"); abort(); } /* Generated stub for master_badmsg */ void master_badmsg(u32 type_expected UNNEEDED, const u8 *msg) diff --git a/gossipd/test/run-find_route-specific.c b/gossipd/test/run-find_route-specific.c index bc40e0398..505e7bd8d 100644 --- a/gossipd/test/run-find_route-specific.c +++ b/gossipd/test/run-find_route-specific.c @@ -43,6 +43,9 @@ bool nannounce_different(struct gossip_store *gs UNNEEDED, /* Generated stub for onion_type_name */ const char *onion_type_name(int e UNNEEDED) { fprintf(stderr, "onion_type_name called!\n"); abort(); } +/* Generated stub for peer_supplied_good_gossip */ +void peer_supplied_good_gossip(struct peer *peer 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) diff --git a/gossipd/test/run-find_route.c b/gossipd/test/run-find_route.c index b80285bb8..09666e605 100644 --- a/gossipd/test/run-find_route.c +++ b/gossipd/test/run-find_route.c @@ -41,6 +41,9 @@ bool nannounce_different(struct gossip_store *gs UNNEEDED, /* Generated stub for onion_type_name */ const char *onion_type_name(int e UNNEEDED) { fprintf(stderr, "onion_type_name called!\n"); abort(); } +/* Generated stub for peer_supplied_good_gossip */ +void peer_supplied_good_gossip(struct peer *peer 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) diff --git a/gossipd/test/run-overlong.c b/gossipd/test/run-overlong.c index 135e0f989..07957390f 100644 --- a/gossipd/test/run-overlong.c +++ b/gossipd/test/run-overlong.c @@ -41,6 +41,9 @@ bool nannounce_different(struct gossip_store *gs UNNEEDED, /* Generated stub for onion_type_name */ const char *onion_type_name(int e UNNEEDED) { fprintf(stderr, "onion_type_name called!\n"); abort(); } +/* Generated stub for peer_supplied_good_gossip */ +void peer_supplied_good_gossip(struct peer *peer 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) diff --git a/gossipd/test/run-txout_failure.c b/gossipd/test/run-txout_failure.c index 258c33b05..04d16a483 100644 --- a/gossipd/test/run-txout_failure.c +++ b/gossipd/test/run-txout_failure.c @@ -53,6 +53,9 @@ bool nannounce_different(struct gossip_store *gs UNNEEDED, /* Generated stub for onion_type_name */ const char *onion_type_name(int e UNNEEDED) { fprintf(stderr, "onion_type_name called!\n"); abort(); } +/* Generated stub for peer_supplied_good_gossip */ +void peer_supplied_good_gossip(struct peer *peer 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)