From ab31f40aa28653d88f3849f5b2ff58e01c26728c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 31 May 2019 17:00:33 +0930 Subject: [PATCH] gossipd: don't charge ourselves fees when calculating route. This means there's now a semantic difference between the default `fromid` and setting `fromid` explicitly to our own node_id. In the default case, it means we don't charge ourselves fees on the route. This means we can spend the full channel balance. We still want to consider the pricing of local channels, however: there's a *reason* to discount one over another, and that is to bias things. So we add the first-hop fee to the *risk* value instead. Signed-off-by: Rusty Russell --- gossipd/gossip_wire.csv | 3 +- gossipd/gossipd.c | 14 ++++++--- gossipd/routing.c | 62 +++++++++++++++++++++++++++---------- lightningd/gossip_control.c | 2 +- 4 files changed, 58 insertions(+), 23 deletions(-) diff --git a/gossipd/gossip_wire.csv b/gossipd/gossip_wire.csv index 575d3ae77..ac740ce09 100644 --- a/gossipd/gossip_wire.csv +++ b/gossipd/gossip_wire.csv @@ -26,7 +26,8 @@ gossip_getnodes_reply,,nodes,num_nodes*struct gossip_getnodes_entry # Pass JSON-RPC getroute call through gossip_getroute_request,3006 -gossip_getroute_request,,source,struct node_id +# Source defaults to "us", and means we don't consider first-hop channel fees +gossip_getroute_request,,source,?struct node_id gossip_getroute_request,,destination,struct node_id gossip_getroute_request,,msatoshi,struct amount_msat # We don't pass doubles, so pass riskfactor * 1000000. diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index 6503c8614..1c30ea715 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -1880,7 +1880,7 @@ static struct io_plan *gossip_init(struct io_conn *conn, static struct io_plan *getroute_req(struct io_conn *conn, struct daemon *daemon, const u8 *msg) { - struct node_id source, destination; + struct node_id *source, destination; struct amount_msat msat; u32 final_cltv; u64 riskfactor_by_million; @@ -1895,7 +1895,12 @@ static struct io_plan *getroute_req(struct io_conn *conn, struct daemon *daemon, * we'll pay), how to trade off more locktime vs. more fees, and how * much cltv we need a the final node to give exact values for each * intermediate hop, as well as how much random fuzz to inject to - * avoid being too predictable. */ + * avoid being too predictable. + * + * We also treat routing slightly differently if we're asking + * for a route from ourselves (the usual case): in that case, + * we don't have to consider fees on our own outgoing channels. + */ if (!fromwire_gossip_getroute_request(msg, msg, &source, &destination, &msat, &riskfactor_by_million, @@ -1905,12 +1910,13 @@ static struct io_plan *getroute_req(struct io_conn *conn, struct daemon *daemon, master_badmsg(WIRE_GOSSIP_GETROUTE_REQUEST, msg); status_trace("Trying to find a route from %s to %s for %s", - type_to_string(tmpctx, struct node_id, &source), + source + ? type_to_string(tmpctx, struct node_id, source) : "(me)", type_to_string(tmpctx, struct node_id, &destination), type_to_string(tmpctx, struct amount_msat, &msat)); /* routing.c does all the hard work; can return NULL. */ - hops = get_route(tmpctx, daemon->rstate, &source, &destination, + hops = get_route(tmpctx, daemon->rstate, source, &destination, msat, riskfactor_by_million / 1000000.0, final_cltv, fuzz, pseudorand_u64(), excluded, max_hops); diff --git a/gossipd/routing.c b/gossipd/routing.c index 7604316bb..93a5ab137 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -517,6 +517,7 @@ static bool fuzz_fee(u64 *fee, * sets newtotal and newrisk */ static bool can_reach(const struct half_chan *c, const struct short_channel_id *scid, + bool no_charge, struct amount_msat total, struct amount_msat risk, double riskfactor, @@ -530,18 +531,29 @@ static bool can_reach(const struct half_chan *c, if (!amount_msat_fee(&fee, total, c->base_fee, c->proportional_fee)) return false; - if (!fuzz_fee(&fee.millisatoshis, scid, fuzz, base_seed)) /* Raw: double manipulation */ + if (!fuzz_fee(&fee.millisatoshis, scid, fuzz, base_seed)) /* Raw: double manipulation */ return false; - if (!amount_msat_add(newtotal, total, fee)) - return false; + if (no_charge) { + *newtotal = total; + + /* We still want to consider the "charge", since it's indicative + * of a bias (we discounted one channel for a reason), but we + * don't pay it. So we count it as additional risk. */ + if (!amount_msat_add(newrisk, risk, fee)) + return false; + } else { + *newrisk = risk; + + if (!amount_msat_add(newtotal, total, fee)) + return false; + } /* Skip a channel if it indicated that it won't route the * requested amount. */ if (!hc_can_carry(c, *newtotal)) return false; - *newrisk = risk; if (!risk_add_fee(newrisk, *newtotal, c->delay, riskfactor, riskbias)) return false; @@ -736,6 +748,7 @@ static void remove_unvisited(struct node *node, struct unvisited *unvisited, static void update_unvisited_neighbors(struct routing_state *rstate, struct node *cur, + const struct node *me, double riskfactor, u64 riskbias, double fuzz, @@ -772,7 +785,9 @@ static void update_unvisited_neighbors(struct routing_state *rstate, continue; } - if (!can_reach(&chan->half[idx], &chan->scid, + /* We're looking at channels *backwards*, so peer == me + * is the right test here for whether we don't charge fees. */ + if (!can_reach(&chan->half[idx], &chan->scid, peer == me, cur->dijkstra.total, cur->dijkstra.risk, riskfactor, riskbias, fuzz, base_seed, &total, &risk)) { @@ -819,6 +834,7 @@ static struct node *first_unvisited(struct unvisited *unvisited) static void dijkstra(struct routing_state *rstate, const struct node *dst, + const struct node *me, double riskfactor, u64 riskbias, double fuzz, const struct siphash_seed *base_seed, @@ -828,7 +844,8 @@ static void dijkstra(struct routing_state *rstate, struct node *cur; while ((cur = first_unvisited(unvisited)) != NULL) { - update_unvisited_neighbors(rstate, cur, riskfactor, riskbias, + update_unvisited_neighbors(rstate, cur, me, + riskfactor, riskbias, fuzz, base_seed, unvisited, costfn); remove_unvisited(cur, unvisited, costfn); if (cur == dst) @@ -842,6 +859,7 @@ static struct chan **build_route(const tal_t *ctx, struct routing_state *rstate, const struct node *from, const struct node *to, + const struct node *me, double riskfactor, u64 riskbias, double fuzz, @@ -888,7 +906,7 @@ static struct chan **build_route(const tal_t *ctx, continue; } - if (!can_reach(hc, &chan->scid, + if (!can_reach(hc, &chan->scid, i == me, peer->dijkstra.total, peer->dijkstra.risk, riskfactor, riskbias, @@ -984,6 +1002,7 @@ static void dijkstra_cleanup(struct unvisited *unvisited) static struct chan ** find_shorter_route(const tal_t *ctx, struct routing_state *rstate, struct node *src, struct node *dst, + const struct node *me, struct amount_msat msat, size_t max_hops, double fuzz, const struct siphash_seed *base_seed, @@ -1017,12 +1036,12 @@ find_shorter_route(const tal_t *ctx, struct routing_state *rstate, SUPERVERBOSE("Running shortest path from %s -> %s", type_to_string(tmpctx, struct node_id, &dst->id), type_to_string(tmpctx, struct node_id, &src->id)); - dijkstra(rstate, dst, riskfactor, 1, fuzz, base_seed, + dijkstra(rstate, dst, NULL, riskfactor, 1, fuzz, base_seed, unvisited, shortest_cost_function); dijkstra_cleanup(unvisited); /* This must succeed, since we found a route before */ - short_route = build_route(ctx, rstate, dst, src, riskfactor, 1, + short_route = build_route(ctx, rstate, dst, src, me, riskfactor, 1, fuzz, base_seed, fee); assert(short_route); if (!amount_msat_sub(&short_cost, @@ -1064,11 +1083,12 @@ find_shorter_route(const tal_t *ctx, struct routing_state *rstate, unvisited = dijkstra_prepare(tmpctx, rstate, src, msat, normal_cost_function); - dijkstra(rstate, dst, riskfactor, riskbias, fuzz, base_seed, + dijkstra(rstate, dst, me, riskfactor, riskbias, fuzz, base_seed, unvisited, normal_cost_function); dijkstra_cleanup(unvisited); - route = build_route(ctx, rstate, dst, src, riskfactor, riskbias, + route = build_route(ctx, rstate, dst, src, me, + riskfactor, riskbias, fuzz, base_seed, &this_fee); SUPERVERBOSE("riskbias %"PRIu64" rlen %zu", @@ -1111,20 +1131,28 @@ find_route(const tal_t *ctx, struct routing_state *rstate, struct amount_msat *fee) { struct node *src, *dst; + const struct node *me; struct unvisited *unvisited; struct chan **route; /* Note: we map backwards, since we know the amount of satoshi we want * at the end, and need to derive how much we need to send. */ - dst = get_node(rstate, from); src = get_node(rstate, to); + /* If from is NULL, that's means it's us. */ + if (!from) + me = dst = get_node(rstate, &rstate->local_id); + else { + dst = get_node(rstate, from); + me = NULL; + } + if (!src) { status_info("find_route: cannot find %s", type_to_string(tmpctx, struct node_id, to)); return NULL; } else if (!dst) { - status_info("find_route: cannot find myself (%s)", + status_info("find_route: cannot find source (%s)", type_to_string(tmpctx, struct node_id, to)); return NULL; } else if (dst == src) { @@ -1135,17 +1163,17 @@ find_route(const tal_t *ctx, struct routing_state *rstate, unvisited = dijkstra_prepare(tmpctx, rstate, src, msat, normal_cost_function); - dijkstra(rstate, dst, riskfactor, 1, fuzz, base_seed, + dijkstra(rstate, dst, me, riskfactor, 1, fuzz, base_seed, unvisited, normal_cost_function); dijkstra_cleanup(unvisited); - route = build_route(ctx, rstate, dst, src, riskfactor, 1, + route = build_route(ctx, rstate, dst, src, me, riskfactor, 1, fuzz, base_seed, fee); if (tal_count(route) <= max_hops) return route; /* This is the far more unlikely case */ - return find_shorter_route(ctx, rstate, src, dst, msat, + return find_shorter_route(ctx, rstate, src, dst, me, msat, max_hops, fuzz, base_seed, route, fee); } @@ -2334,7 +2362,7 @@ struct route_hop *get_route(const tal_t *ctx, struct routing_state *rstate, total_delay += c->delay; n = other_node(n, route[i]); } - assert(node_id_eq(&n->id, source)); + assert(node_id_eq(&n->id, source ? source : &rstate->local_id)); return hops; } diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index d0380a2d0..b8d1bdba6 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -307,7 +307,7 @@ static struct command_result *json_getroute(struct command *cmd, p_req("msatoshi", param_msat, &msat), p_req("riskfactor", param_double, &riskfactor), p_opt_def("cltv", param_number, &cltv, 9), - p_opt_def("fromid", param_node_id, &source, ld->id), + p_opt("fromid", param_node_id, &source), p_opt_def("fuzzpercent", param_percent, &fuzz, 5.0), p_opt("exclude", param_array, &excludetok), p_opt_def("maxhops", param_number, &max_hops,