From b4aff493f1aacd2c1d0a41ca5771f9ade3a16903 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sun, 26 Jul 2020 10:30:14 +0930 Subject: [PATCH] pay: always send onion error message to gossipd. There were no channel updates in my log; because sendonion doesn't know the actual node_ids or channel_ids, we can't tell gossipd what node/channel it was so it can no longer remove them on PERM errors. However, we can tell it the error message so it can apply the update. Fixes: #3877 Signed-off-by: Rusty Russell --- gossipd/gossip_wire.csv | 3 -- gossipd/gossipd.c | 28 +++++-------- gossipd/routing.c | 90 ----------------------------------------- gossipd/routing.h | 7 ---- lightningd/pay.c | 18 +++------ 5 files changed, 17 insertions(+), 129 deletions(-) diff --git a/gossipd/gossip_wire.csv b/gossipd/gossip_wire.csv index 60a638fa5..9231fd764 100644 --- a/gossipd/gossip_wire.csv +++ b/gossipd/gossip_wire.csv @@ -98,9 +98,6 @@ msgdata,gossip_get_txout_reply,outscript,u8,len # master->gossipd an htlc failed with this onion error. msgtype,gossip_payment_failure,3021 -msgdata,gossip_payment_failure,erring_node,node_id, -msgdata,gossip_payment_failure,erring_channel,short_channel_id, -msgdata,gossip_payment_failure,erring_channel_direction,u8, msgdata,gossip_payment_failure,len,u16, msgdata,gossip_payment_failure,error,u8,len diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index 257c25a5d..c838d4e33 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -1506,32 +1506,26 @@ static struct io_plan *handle_payment_failure(struct io_conn *conn, struct daemon *daemon, const u8 *msg) { - struct node_id erring_node; - struct short_channel_id erring_channel; - u8 erring_channel_direction; u8 *error; - enum onion_type failcode; u8 *channel_update; - if (!fromwire_gossip_payment_failure(msg, msg, - &erring_node, - &erring_channel, - &erring_channel_direction, - &error)) + if (!fromwire_gossip_payment_failure(msg, msg, &error)) master_badmsg(WIRE_GOSSIP_PAYMENT_FAILURE, msg); - failcode = fromwire_peektype(error); channel_update = channel_update_from_onion_error(tmpctx, error); - if (channel_update) + if (channel_update) { status_debug("Extracted channel_update %s from onionreply %s", tal_hex(tmpctx, channel_update), tal_hex(tmpctx, error)); - routing_failure(daemon->rstate, - &erring_node, - &erring_channel, - erring_channel_direction, - failcode, - channel_update); + u8 *err = handle_channel_update(daemon->rstate, channel_update, + NULL, NULL); + if (err) { + status_info("extracted bad channel_update %s from onionreply %s", + sanitize_error(err, err, NULL), + error); + tal_free(err); + } + } return daemon_conn_read_next(conn, daemon->master); } diff --git a/gossipd/routing.c b/gossipd/routing.c index 38ed32c18..f347835f5 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -2762,96 +2762,6 @@ struct route_hop **get_route(const tal_t *ctx, struct routing_state *rstate, return hops; } -void routing_failure(struct routing_state *rstate, - const struct node_id *erring_node_id, - const struct short_channel_id *scid, - int erring_direction, - enum onion_type failcode, - const u8 *channel_update) -{ - struct chan **pruned = tal_arr(tmpctx, struct chan *, 0); - - status_debug("Received routing failure 0x%04x (%s), " - "erring node %s, " - "channel %s/%u", - (int) failcode, onion_type_name(failcode), - type_to_string(tmpctx, struct node_id, erring_node_id), - type_to_string(tmpctx, struct short_channel_id, scid), - erring_direction); - - /* lightningd will only extract this if UPDATE is set. */ - if (channel_update) { - u8 *err = handle_channel_update(rstate, channel_update, - NULL, NULL); - if (err) { - status_unusual("routing_failure: " - "bad channel_update %s", - sanitize_error(err, err, NULL)); - tal_free(err); - } - } else if (failcode & UPDATE) { - status_unusual("routing_failure: " - "UPDATE bit set, no channel_update. " - "failcode: 0x%04x", - (int) failcode); - } - - /* We respond to permanent errors, ignore the rest: they're - * for the pay command to worry about. */ - if (!(failcode & PERM)) - return; - - if (failcode & NODE) { - struct node *node = get_node(rstate, erring_node_id); - if (!node) { - status_unusual("routing_failure: Erring node %s not in map", - type_to_string(tmpctx, struct node_id, - erring_node_id)); - } else { - struct chan_map_iter i; - struct chan *c; - - status_debug("Deleting node %s", - type_to_string(tmpctx, - struct node_id, - &node->id)); - for (c = first_chan(node, &i); c; c = next_chan(node, &i)) { - /* Set it up to be pruned. */ - tal_arr_expand(&pruned, c); - } - } - } else { - struct chan *chan = get_channel(rstate, scid); - - if (!chan) - status_unusual("routing_failure: " - "Channel %s unknown", - type_to_string(tmpctx, - struct short_channel_id, - scid)); - else { - /* This error can be triggered by sendpay if caller - * uses the wrong key for dest. */ - if (failcode == WIRE_INVALID_ONION_HMAC - && !node_id_eq(&chan->nodes[!erring_direction]->id, - erring_node_id)) - return; - - status_debug("Deleting channel %s", - type_to_string(tmpctx, - struct short_channel_id, - scid)); - /* Set it up to be deleted. */ - tal_arr_expand(&pruned, chan); - } - } - - /* Now free all the chans and maybe even nodes. */ - for (size_t i = 0; i < tal_count(pruned); i++) - free_chan(rstate, pruned[i]); -} - - void route_prune(struct routing_state *rstate) { u64 now = gossip_time_now(rstate).ts.tv_sec; diff --git a/gossipd/routing.h b/gossipd/routing.h index 3e3eb1b4e..186a48652 100644 --- a/gossipd/routing.h +++ b/gossipd/routing.h @@ -424,13 +424,6 @@ struct route_hop **get_route(const tal_t *ctx, struct routing_state *rstate, u64 seed, struct exclude_entry **excluded, u32 max_hops); -/* Disable channel(s) based on the given routing failure. */ -void routing_failure(struct routing_state *rstate, - const struct node_id *erring_node, - const struct short_channel_id *erring_channel, - int erring_direction, - enum onion_type failcode, - const u8 *channel_update); void route_prune(struct routing_state *rstate); diff --git a/lightningd/pay.c b/lightningd/pay.c index 1322ec06b..5c3e87254 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -437,8 +437,6 @@ remote_routing_failure(const tal_t *ctx, *pay_errcode = PAY_TRY_OTHER_ROUTE; erring_node = &route_nodes[origin_index]; } else { - u8 *gossip_msg; - *pay_errcode = PAY_TRY_OTHER_ROUTE; /* Report the *next* channel as failing. */ @@ -455,18 +453,14 @@ remote_routing_failure(const tal_t *ctx, erring_node = &route_nodes[origin_index + 1]; } else erring_node = &route_nodes[origin_index]; - - /* Tell gossipd: it may want to remove channels or even nodes - * in response to this, and there may be a channel_update - * embedded too */ - gossip_msg = towire_gossip_payment_failure(NULL, - erring_node, - erring_channel, - dir, - failuremsg); - subd_send_msg(ld->gossip, take(gossip_msg)); } + /* Tell gossipd; it will try to extract channel_update */ + /* FIXME: sendonion caller should do this, and inform gossipd of any + * permanent errors. */ + subd_send_msg(ld->gossip, + take(towire_gossip_payment_failure(NULL, failuremsg))); + routing_failure->erring_index = (unsigned int) (origin_index + 1); routing_failure->failcode = failcode; routing_failure->msg = tal_dup_talarr(routing_failure, u8, failuremsg);