From cda8f8190b67504b0fba683381533ca21e8022c0 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 15 Jun 2021 06:37:38 +0930 Subject: [PATCH] invoice: overhaul routehints to use topology.listincoming, cleanup. This turned into a more extensive cleanup than intended. The previous warnings were overlapping and confusing, especially now MPP is the norm. *warning_capacity* is now the "even under best circumstances, we don't have enough incoming capacity", which is really what warning_mpp_capacity was trying to say (no longer printed). *warning_offline* and *warning_deadends* are only given if adding such peers would have helped give capacity (i.e. not if *warning_capacity* is set). The new *warning_private_unused* tells you that we would have sufficient capacity, but we refused to expose private channels. The test cases have been enhanced to cover the new warnings. Signed-off-by: Rusty Russell Changelog-Added: JSON-RPC: `invoice` now gives `warning_private_unused` if unused unannounced channels could have provided sufficient capacity. Changelog-Changed: JSON-RPC: `invoice` warnings are now better defined, and `warning_mpp_capacity` is no longer included (since `warning_capacity` covers that). --- doc/lightning-invoice.7 | 15 +- doc/lightning-invoice.7.md | 11 +- lightningd/invoice.c | 337 +++++++++++++------- lightningd/routehint.c | 258 ++++++++++----- lightningd/routehint.h | 21 +- lightningd/test/run-invoice-select-inchan.c | 39 ++- tests/test_invoices.py | 35 +- 7 files changed, 484 insertions(+), 232 deletions(-) diff --git a/doc/lightning-invoice.7 b/doc/lightning-invoice.7 index 9e07b6ac1..d640a134f 100644 --- a/doc/lightning-invoice.7 +++ b/doc/lightning-invoice.7 @@ -106,19 +106,16 @@ One of the following warnings may occur (on success): .RS .IP \[bu] -\fIwarning_offline\fR if no channel with a currently connected peer has - the incoming capacity to pay this invoice +\fIwarning_capacity\fR: even using all possible channels, there's not enough incoming capacity to pay this invoice\. .IP \[bu] -\fIwarning_capacity\fR if there is no channel that has sufficient - incoming capacity +\fIwarning_offline\fR: there would be enough incoming capacity, but some channels are offline, so there isn't\. .IP \[bu] -\fIwarning_deadends\fR if there is no channel that is not a dead-end +\fIwarning_deadends\fR: there would be enough incoming capacity, but some channels are dead-ends (no other public channels from those peers), so there isn't\. +.IP \[bu] +\fIwarning_private_unused\fR: there would be enough incoming capacity, but some channels are unannounced and \fIexposeprivatechannels\fR is \fIfalse\fR, so there isn't\. .IP \[bu] \fIwarning_mpp\fR if there is sufficient capacity, but not in a single channel, so the payer will have to use multi-part payments\. -.IP \[bu] -\fIwarning_mpp_capacity\fR if there is not sufficient capacity, even with all - channels\. .RE .SH AUTHOR @@ -134,4 +131,4 @@ Rusty Russell \fI is mainly responsible\. Main web site: \fIhttps://github.com/ElementsProject/lightning\fR -\" SHA256STAMP:dc375ee583188c669574a8ef203971df650ef221567dccb8df7a81ed5ee4990f +\" SHA256STAMP:d53ec67cd81a41c7218e282c3d7662933868b25190334e9322de8a90ab99d603 diff --git a/doc/lightning-invoice.7.md b/doc/lightning-invoice.7.md index 6f43a9b32..d48246c68 100644 --- a/doc/lightning-invoice.7.md +++ b/doc/lightning-invoice.7.md @@ -87,15 +87,12 @@ The following error codes may occur: - 902: None of the specified *exposeprivatechannels* were usable. One of the following warnings may occur (on success): -- *warning\_offline* if no channel with a currently connected peer has - the incoming capacity to pay this invoice -- *warning\_capacity* if there is no channel that has sufficient - incoming capacity -- *warning\_deadends* if there is no channel that is not a dead-end +- *warning_capacity*: even using all possible channels, there's not enough incoming capacity to pay this invoice. +- *warning_offline*: there would be enough incoming capacity, but some channels are offline, so there isn't. +- *warning_deadends*: there would be enough incoming capacity, but some channels are dead-ends (no other public channels from those peers), so there isn't. +- *warning_private_unused*: there would be enough incoming capacity, but some channels are unannounced and *exposeprivatechannels* is *false*, so there isn't. - *warning_mpp* if there is sufficient capacity, but not in a single channel, so the payer will have to use multi-part payments. -- *warning_mpp_capacity* if there is not sufficient capacity, even with all - channels. AUTHOR ------ diff --git a/lightningd/invoice.c b/lightningd/invoice.c index 56b42a10e..bf7c2ec4c 100644 --- a/lightningd/invoice.c +++ b/lightningd/invoice.c @@ -603,8 +603,7 @@ static struct route_info **select_inchan_mpp(const tal_t *ctx, struct lightningd *ld, struct amount_msat amount_needed, struct routehint_candidate - *candidates, - bool *warning_mpp_capacity) + *candidates) { /* The total amount we have gathered for incoming channels. */ struct amount_msat gathered; @@ -637,9 +636,6 @@ static struct route_info **select_inchan_mpp(const tal_t *ctx, candidates[i].c->rr_number = ld->rr_counter++; } - /* Check if we gathered enough. */ - *warning_mpp_capacity = amount_msat_less(gathered, amount_needed); - return routehints; } @@ -657,76 +653,159 @@ struct invoice_info { struct chanhints *chanhints; }; -static void gossipd_incoming_channels_reply(struct subd *gossipd, - const u8 *msg, - const int *fs, - struct invoice_info *info) +/* Add routehints based on listincoming results: NULL means success. */ +static struct command_result * +add_routehints(struct invoice_info *info, + const char *buffer, + const jsmntok_t *toks, + bool *warning_mpp, + bool *warning_capacity, + bool *warning_deadends, + bool *warning_offline, + bool *warning_private_unused) +{ + const struct chanhints *chanhints = info->chanhints; + bool node_unpublished; + struct amount_msat avail_capacity, deadend_capacity, offline_capacity, + private_capacity; + struct routehint_candidate *candidates; + struct amount_msat total, needed; + + /* Dev code can force routes. */ + if (tal_count(info->b11->routes) != 0) { + *warning_mpp = *warning_capacity = *warning_deadends + = *warning_offline = *warning_private_unused + = false; + return NULL; + } + + candidates = routehint_candidates(tmpctx, info->cmd->ld, + buffer, toks, + chanhints ? &chanhints->expose_all_private : NULL, + chanhints ? chanhints->hints : NULL, + &node_unpublished, + &avail_capacity, + &private_capacity, + &deadend_capacity, + &offline_capacity); + + /* If they told us to use scids and we couldn't, fail. */ + if (tal_count(candidates) == 0 + && chanhints && tal_count(chanhints->hints) != 0) { + return command_fail(info->cmd, + INVOICE_HINTS_GAVE_NO_ROUTES, + "None of those hints were suitable local channels"); + } + + needed = info->b11->msat ? *info->b11->msat : AMOUNT_MSAT(1); + + /* If we are not completely unpublished, try with reservoir + * sampling first. + * + * Why do we not do this if we are completely unpublished? + * Because it is possible that multiple invoices will, by + * chance, select the same channel as routehint. + * This single channel might not be able to accept all the + * incoming payments on all the invoices generated. + * If we were published, that is fine because the payer can + * fall back to just attempting to route directly. + * But if we were unpublished, the only way for the payer to + * reach us would be via the routehints we provide, so we + * should make an effort to avoid overlapping incoming + * channels, which is done by select_inchan_mpp. + */ + if (!node_unpublished) + info->b11->routes = select_inchan(info->b11, + info->cmd->ld, + needed, + candidates); + + /* If we are completely unpublished, or if the above reservoir + * sampling fails, select channels by round-robin. */ + if (tal_count(info->b11->routes) == 0) { + info->b11->routes = select_inchan_mpp(info->b11, + info->cmd->ld, + needed, + candidates); + *warning_mpp = (tal_count(info->b11->routes) > 1); + } else { + *warning_mpp = false; + } + + log_debug(info->cmd->ld->log, "needed = %s, avail_capacity = %s, private_capacity = %s, offline_capacity = %s, deadend_capacity = %s", + type_to_string(tmpctx, struct amount_msat, &needed), + type_to_string(tmpctx, struct amount_msat, &avail_capacity), + type_to_string(tmpctx, struct amount_msat, &private_capacity), + type_to_string(tmpctx, struct amount_msat, &offline_capacity), + type_to_string(tmpctx, struct amount_msat, &deadend_capacity)); + + if (!amount_msat_add(&total, avail_capacity, offline_capacity) + || !amount_msat_add(&total, total, deadend_capacity) + || !amount_msat_add(&total, total, private_capacity)) + fatal("Cannot add %s + %s + %s + %s", + type_to_string(tmpctx, struct amount_msat, + &avail_capacity), + type_to_string(tmpctx, struct amount_msat, + &offline_capacity), + type_to_string(tmpctx, struct amount_msat, + &deadend_capacity), + type_to_string(tmpctx, struct amount_msat, + &private_capacity)); + + /* If we literally didn't have capacity at all, warn. */ + *warning_capacity = amount_msat_greater_eq(needed, total); + + /* We only warn about these if we didn't have capacity and + * they would have helped. */ + *warning_offline = false; + *warning_deadends = false; + *warning_private_unused = false; + if (amount_msat_greater(needed, avail_capacity)) { + struct amount_msat tot; + + /* We didn't get enough: would offline have helped? */ + if (!amount_msat_add(&tot, avail_capacity, offline_capacity)) + abort(); + if (amount_msat_greater_eq(tot, needed)) { + *warning_offline = true; + goto done; + } + + /* Hmm, what about deadends? */ + if (!amount_msat_add(&tot, tot, deadend_capacity)) + abort(); + if (amount_msat_greater_eq(tot, needed)) { + *warning_deadends = true; + goto done; + } + + /* What about private channels? */ + if (!amount_msat_add(&tot, tot, private_capacity)) + abort(); + if (amount_msat_greater_eq(tot, needed)) { + *warning_private_unused = true; + goto done; + } + } + +done: + return NULL; +} + +static struct command_result * +invoice_complete(struct invoice_info *info, + bool warning_no_listincoming, + bool warning_mpp, + bool warning_capacity, + bool warning_deadends, + bool warning_offline, + bool warning_private_unused) { struct json_stream *response; struct invoice invoice; char *b11enc; const struct invoice_details *details; struct wallet *wallet = info->cmd->ld->wallet; - const struct chanhints *chanhints = info->chanhints; - - struct routehint_candidate *candidates; - struct amount_msat offline_amt; - bool warning_mpp = false; - bool warning_mpp_capacity = false; - bool deadends; - bool node_unpublished; - - candidates = routehint_candidates(tmpctx, info->cmd->ld, msg, - chanhints ? chanhints->expose_all_private : false, - chanhints ? chanhints->hints : NULL, - &node_unpublished, - &deadends, - &offline_amt); - - /* If they told us to use scids and we couldn't, fail. */ - if (tal_count(candidates) == 0 - && chanhints && tal_count(chanhints->hints) != 0) { - was_pending(command_fail(info->cmd, - INVOICE_HINTS_GAVE_NO_ROUTES, - "None of those hints were suitable local channels")); - return; - } - - if (tal_count(info->b11->routes) == 0) { - struct amount_msat needed; - needed = info->b11->msat ? *info->b11->msat : AMOUNT_MSAT(1); - - /* If we are not completely unpublished, try with reservoir - * sampling first. - * - * Why do we not do this if we are completely unpublished? - * Because it is possible that multiple invoices will, by - * chance, select the same channel as routehint. - * This single channel might not be able to accept all the - * incoming payments on all the invoices generated. - * If we were published, that is fine because the payer can - * fall back to just attempting to route directly. - * But if we were unpublished, the only way for the payer to - * reach us would be via the routehints we provide, so we - * should make an effort to avoid overlapping incoming - * channels, which is done by select_inchan_mpp. - */ - if (!node_unpublished) - info->b11->routes = select_inchan(info->b11, - info->cmd->ld, - needed, - candidates); - /* If we are completely unpublished, or if the above reservoir - * sampling fails, select channels by round-robin. */ - if (tal_count(info->b11->routes) == 0) { - info->b11->routes = select_inchan_mpp(info->b11, - info->cmd->ld, - needed, - candidates, - &warning_mpp_capacity); - warning_mpp = (tal_count(info->b11->routes) > 1); - } - } b11enc = bolt11_encode(info, info->b11, false, hsm_sign_b11, info->cmd->ld); @@ -734,10 +813,9 @@ static void gossipd_incoming_channels_reply(struct subd *gossipd, /* Check duplicate preimage (unlikely unless they specified it!) */ if (wallet_invoice_find_by_rhash(wallet, &invoice, &info->b11->payment_hash)) { - was_pending(command_fail(info->cmd, - INVOICE_PREIMAGE_ALREADY_EXISTS, - "preimage already used")); - return; + return command_fail(info->cmd, + INVOICE_PREIMAGE_ALREADY_EXISTS, + "preimage already used"); } if (!wallet_invoice_create(wallet, @@ -751,10 +829,9 @@ static void gossipd_incoming_channels_reply(struct subd *gossipd, &info->payment_preimage, &info->b11->payment_hash, NULL)) { - was_pending(command_fail(info->cmd, INVOICE_LABEL_ALREADY_EXISTS, - "Duplicate label '%s'", - info->label->s)); - return; + return command_fail(info->cmd, INVOICE_LABEL_ALREADY_EXISTS, + "Duplicate label '%s'", + info->label->s); } /* Get details */ @@ -768,41 +845,60 @@ static void gossipd_incoming_channels_reply(struct subd *gossipd, notify_invoice_creation(info->cmd->ld, info->b11->msat, info->payment_preimage, info->label); - /* Warn if there's not sufficient incoming capacity. */ - if (tal_count(info->b11->routes) == 0) { - log_unusual(info->cmd->ld->log, - "invoice: insufficient incoming capacity for %s%s", - info->b11->msat - ? type_to_string(tmpctx, struct amount_msat, - info->b11->msat) - : "0", - amount_msat_greater(offline_amt, AMOUNT_MSAT(0)) - ? " (among currently connected peers)" : ""); - - if (amount_msat_greater(offline_amt, AMOUNT_MSAT(0))) { - json_add_string(response, "warning_offline", - "No channel with a peer that is currently connected" - " has sufficient incoming capacity"); - } else if (deadends) { - json_add_string(response, "warning_deadends", - "No channel with a peer that is not a dead end"); - } else if (tal_count(candidates) == 0) { - json_add_string(response, "warning_capacity", - "No channels"); - } else { - json_add_string(response, "warning_capacity", - "No channel with a peer that has sufficient incoming capacity"); - } - } - + if (warning_no_listincoming) + json_add_string(response, "warning_listincoming", + "No listincoming command available, cannot add routehints to invoice"); if (warning_mpp) json_add_string(response, "warning_mpp", "The invoice might only be payable by MPP-capable payers."); - if (warning_mpp_capacity) - json_add_string(response, "warning_mpp_capacity", - "The total incoming capacity is still insufficient even if the payer had MPP capability."); + if (warning_capacity) + json_add_string(response, "warning_capacity", + "Insufficient incoming channel capacity to pay invoice"); - was_pending(command_success(info->cmd, response)); + if (warning_deadends) + json_add_string(response, "warning_deadends", + "Insufficient incoming capacity, once dead-end peers were excluded"); + + if (warning_offline) + json_add_string(response, "warning_offline", + "Insufficient incoming capacity, once offline peers were excluded"); + + if (warning_private_unused) + json_add_string(response, "warning_private_unused", + "Insufficient incoming capacity, once private channels were excluded (try exposeprivatechannels=true?)"); + + return command_success(info->cmd, response); +} + +/* Return from "listincoming". */ +static void listincoming_done(const char *buffer, + const jsmntok_t *toks, + const jsmntok_t *idtok UNUSED, + struct invoice_info *info) +{ + struct lightningd *ld = info->cmd->ld; + struct command_result *ret; + bool warning_mpp, warning_capacity, warning_deadends, warning_offline, warning_private_unused; + + ret = add_routehints(info, buffer, toks, + &warning_mpp, + &warning_capacity, + &warning_deadends, + &warning_offline, + &warning_private_unused); + if (ret) + return; + + /* We're actually outside a db transaction here: spooky! */ + db_begin_transaction(ld->wallet->db); + invoice_complete(info, + false, + warning_mpp, + warning_capacity, + warning_deadends, + warning_offline, + warning_private_unused); + db_commit_transaction(ld->wallet->db); } #if DEVELOPER @@ -940,12 +1036,11 @@ static struct command_result *param_chanhints(struct command *cmd, /* Could be simply "true" or "false" */ if (json_to_bool(buffer, tok, &boolhint)) { (*chanhints)->expose_all_private = boolhint; - (*chanhints)->hints - = tal_arr(*chanhints, struct short_channel_id, 0); + (*chanhints)->hints = NULL; return NULL; } - (*chanhints)->expose_all_private = false; + (*chanhints)->expose_all_private = true; /* Could be a single short_channel_id or an array */ if (tok->type == JSMN_ARRAY) { size_t i; @@ -999,6 +1094,8 @@ static struct command_result *json_invoice(struct command *cmd, struct secret payment_secret; struct preimage *preimage; u32 *cltv; + struct jsonrpc_request *req; + struct plugin *plugin; #if DEVELOPER const jsmntok_t *routes; #endif @@ -1094,11 +1191,21 @@ static struct command_result *json_invoice(struct command *cmd, if (fallback_scripts) info->b11->fallbacks = tal_steal(info->b11, fallback_scripts); - subd_req(cmd, cmd->ld->gossip, - take(towire_gossipd_get_incoming_channels(NULL)), - -1, 0, gossipd_incoming_channels_reply, info); + req = jsonrpc_request_start(info, "listincoming", + cmd->ld->log, + NULL, listincoming_done, + info); + jsonrpc_request_end(req); - return command_still_pending(cmd); + plugin = find_plugin_for_command(cmd->ld, "listincoming"); + if (plugin) { + plugin_request_send(plugin, req); + return command_still_pending(cmd); + } + + /* We can't generate routehints without listincoming. */ + return invoice_complete(info, true, + false, false, false, false, false); } static const struct json_command invoice_command = { diff --git a/lightningd/routehint.c b/lightningd/routehint.c index 284dedcae..d87d0b73d 100644 --- a/lightningd/routehint.c +++ b/lightningd/routehint.c @@ -1,26 +1,12 @@ #include +#include #include #include +#include #include #include #include -static void append_routes(struct route_info **dst, const struct route_info *src) -{ - size_t n = tal_count(*dst); - - tal_resize(dst, n + tal_count(src)); - memcpy(*dst + n, src, tal_count(src) * sizeof(*src)); -} - -static void append_bools(bool **dst, const bool *src) -{ - size_t n = tal_count(*dst); - - tal_resize(dst, n + tal_count(src)); - memcpy(*dst + n, src, tal_count(src) * sizeof(*src)); -} - static bool scid_in_arr(const struct short_channel_id *scidarr, const struct short_channel_id *scid) { @@ -34,100 +20,204 @@ static bool scid_in_arr(const struct short_channel_id *scidarr, struct routehint_candidate * routehint_candidates(const tal_t *ctx, struct lightningd *ld, - const u8 *incoming_channels_reply, - bool expose_all_private, + const char *buf, + const jsmntok_t *toks, + const bool *expose_all_private, const struct short_channel_id *hints, bool *none_public, - bool *deadends, - struct amount_msat *amount_offline) + struct amount_msat *avail_capacity, + struct amount_msat *private_capacity, + struct amount_msat *deadend_capacity, + struct amount_msat *offline_capacity) { - struct routehint_candidate *candidates; - struct route_info *inchans, *private; - bool *inchan_deadends, *private_deadends; + struct routehint_candidate *candidates, *privcandidates; + const jsmntok_t *t, *arr; + size_t i; - if (!fromwire_gossipd_get_incoming_channels_reply(tmpctx, - incoming_channels_reply, - &inchans, - &inchan_deadends, - &private, - &private_deadends)) - fatal("Gossip gave bad GOSSIPD_GET_INCOMING_CHANNELS_REPLY %s", - tal_hex(tmpctx, incoming_channels_reply)); + log_debug(ld->log, "routehint: %.*s", + json_tok_full_len(toks), + json_tok_full(buf, toks)); - *none_public = (tal_count(inchans) == 0) && (tal_count(private) > 0); - *deadends = false; - - /* fromwire explicitly makes empty arrays into NULL */ - if (!inchans) { - inchans = tal_arr(tmpctx, struct route_info, 0); - inchan_deadends = tal_arr(tmpctx, bool, 0); - } - - if (expose_all_private) { - append_routes(&inchans, private); - append_bools(&inchan_deadends, private_deadends); - } else if (hints) { - /* Start by considering all channels as candidates */ - append_routes(&inchans, private); - append_bools(&inchan_deadends, private_deadends); - - /* Consider only hints they gave */ - for (size_t i = 0; i < tal_count(inchans); i++) { - if (!scid_in_arr(hints, - &inchans[i].short_channel_id)) { - tal_arr_remove(&inchans, i); - tal_arr_remove(&inchan_deadends, i); - i--; - } else - /* If they specify directly, we don't - * care if it's a deadend */ - inchan_deadends[i] = false; - } - } else { - assert(!hints); - /* By default, only consider private channels if there are - * no public channels *at all* */ - if (tal_count(inchans) == 0) { - append_routes(&inchans, private); - append_bools(&inchan_deadends, private_deadends); - } - } + /* We get the full JSON, including result. */ + t = json_get_member(buf, toks, "result"); + if (!t) + fatal("Missing result from listincoming: %.*s", + json_tok_full_len(toks), + json_tok_full(buf, toks)); + arr = json_get_member(buf, t, "incoming"); candidates = tal_arr(ctx, struct routehint_candidate, 0); - *amount_offline = AMOUNT_MSAT(0); + privcandidates = tal_arr(tmpctx, struct routehint_candidate, 0); + *none_public = true; + *deadend_capacity = AMOUNT_MSAT(0); + *offline_capacity = AMOUNT_MSAT(0); + *avail_capacity = AMOUNT_MSAT(0); + *private_capacity = AMOUNT_MSAT(0); - for (size_t i = 0; i < tal_count(inchans); i++) { - struct peer *peer; + /* We combine the JSON output which knows the peers' details, + * with our internal information */ + json_for_each_arr(i, t, arr) { + struct amount_msat capacity; + const char *err; struct routehint_candidate candidate; + struct amount_msat fee_base; + struct route_info *r; + struct peer *peer; + bool is_public; + + r = tal(tmpctx, struct route_info); + + err = json_scan(tmpctx, buf, t, + "{id:%" + ",short_channel_id:%" + ",fee_base_msat:%" + ",fee_proportional_millionths:%" + ",cltv_expiry_delta:%" + ",incoming_capacity_msat:%" + "}", + JSON_SCAN(json_to_node_id, &r->pubkey), + JSON_SCAN(json_to_short_channel_id, + &r->short_channel_id), + JSON_SCAN(json_to_msat, &fee_base), + JSON_SCAN(json_to_u32, + &r->fee_proportional_millionths), + JSON_SCAN(json_to_u16, &r->cltv_expiry_delta), + JSON_SCAN(json_to_msat, &capacity)); + + if (err) { + fatal("Invalid return from listincoming (%s): %.*s", + err, + json_tok_full_len(toks), + json_tok_full(buf, toks)); + } /* Do we know about this peer? */ - peer = peer_by_id(ld, &inchans[i].pubkey); - if (!peer) + peer = peer_by_id(ld, &r->pubkey); + if (!peer) { + log_debug(ld->log, "%s: unknown peer", + type_to_string(tmpctx, + struct short_channel_id, + &r->short_channel_id)); continue; + } /* Does it have a channel in state CHANNELD_NORMAL */ candidate.c = peer_normal_channel(peer); - if (!candidate.c) - continue; - - /* Is it a dead-end? */ - if (inchan_deadends[i]) { - *deadends = true; + if (!candidate.c) { + log_debug(ld->log, "%s: abnormal channel", + type_to_string(tmpctx, + struct short_channel_id, + &r->short_channel_id)); continue; } candidate.capacity = channel_amount_receivable(candidate.c); + /* Now we can tell if it's public. If so (even if it's otherwise + * unusable), we *don't* expose private channels! */ + is_public = (candidate.c->channel_flags + & CHANNEL_FLAGS_ANNOUNCE_CHANNEL); + + if (is_public) + *none_public = false; + + /* If they explicitly say to expose all private ones, consider + * it public. */ + if (expose_all_private != NULL && *expose_all_private) + is_public = true; + + r->fee_base_msat = fee_base.millisatoshis; /* Raw: route_info */ + /* Could wrap: if so ignore */ + if (!amount_msat_eq(amount_msat(r->fee_base_msat), fee_base)) { + log_debug(ld->log, + "Peer charging insane fee %.*s; ignoring", + json_tok_full_len(t), + json_tok_full(buf, t)); + continue; + } + + /* Consider only hints they gave */ + if (hints) { + log_debug(ld->log, "We have hints!"); + if (!scid_in_arr(hints, &r->short_channel_id)) { + log_debug(ld->log, "scid %s not in hints", + type_to_string(tmpctx, + struct short_channel_id, + &r->short_channel_id)); + continue; + } + /* If they give us a hint, we use even if capacity 0 */ + } else if (amount_msat_eq(capacity, AMOUNT_MSAT(0))) { + log_debug(ld->log, "%s: deadend", + type_to_string(tmpctx, + struct short_channel_id, + &r->short_channel_id)); + if (!amount_msat_add(deadend_capacity, + *deadend_capacity, + candidate.capacity)) + fatal("Overflow summing deadend capacity!"); + continue; + } + /* Is it offline? */ if (candidate.c->owner == NULL) { - if (!amount_msat_add(amount_offline, - *amount_offline, + log_debug(ld->log, "%s: offline", + type_to_string(tmpctx, + struct short_channel_id, + &r->short_channel_id)); + if (!amount_msat_add(offline_capacity, + *offline_capacity, candidate.capacity)) fatal("Overflow summing offline capacity!"); continue; } - candidate.r = &inchans[i]; - tal_arr_expand(&candidates, candidate); + + /* OK, finish it and append to one of the arrays. */ + if (is_public) { + log_debug(ld->log, "%s: added to public", + type_to_string(tmpctx, + struct short_channel_id, + &r->short_channel_id)); + candidate.r = tal_steal(candidates, r); + tal_arr_expand(&candidates, candidate); + if (!amount_msat_add(avail_capacity, + *avail_capacity, + candidate.capacity)) { + fatal("Overflow summing pub capacities %s + %s", + type_to_string(tmpctx, struct amount_msat, + avail_capacity), + type_to_string(tmpctx, struct amount_msat, + &candidate.capacity)); + } + } else { + log_debug(ld->log, "%s: added to private", + type_to_string(tmpctx, + struct short_channel_id, + &r->short_channel_id)); + candidate.r = tal_steal(privcandidates, r); + tal_arr_expand(&privcandidates, candidate); + if (!amount_msat_add(private_capacity, + *private_capacity, + candidate.capacity)) { + fatal("Overflow summing priv capacities %s + %s", + type_to_string(tmpctx, struct amount_msat, + private_capacity), + type_to_string(tmpctx, struct amount_msat, + &candidate.capacity)); + } + } + } + + /* By default, only consider private channels if there are + * no public channels *at all* */ + if (expose_all_private == NULL + && tal_count(candidates) == 0 && *none_public) { + log_debug(ld->log, "No publics: using private channels"); + tal_free(candidates); + candidates = tal_steal(ctx, privcandidates); + *avail_capacity = *private_capacity; + /* This reflects *unused* private capacity. */ + *private_capacity = AMOUNT_MSAT(0); } return candidates; diff --git a/lightningd/routehint.h b/lightningd/routehint.h index 8eb8476ec..27eadca68 100644 --- a/lightningd/routehint.h +++ b/lightningd/routehint.h @@ -20,21 +20,26 @@ struct routehint_candidate { * routehint_candidates - get possible incoming channels for routehinting. * @ctx: tal context to allocate return off * @ld: lightningd - * @incoming_channels_reply: reply from gossipd get_incoming_channels - * @expose_all_private: consider private channels too (otherwise iff no public) + * @buf, @toks: output of listincoming command + * @expose_all_private: trinary. NULL=iff no public, true=always, false=never. * @hints: only consider these channels (if !expose_all_private). * @none_public: set to true if we used private channels because none were public. - * @deadends: set to true if we found a dead-end channel. - * @amount_offline: amount we didn't consider due to offline channels. + * @avail_capacity: total capacity of usable channels. + * @private_capacity: total capacity of unused private channels. + * @deadend_capacity: total capacity of "deadend" channels. + * @offline_capacity: total capacity of offline channels. */ struct routehint_candidate * routehint_candidates(const tal_t *ctx, struct lightningd *ld, - const u8 *incoming_channels_reply, - bool expose_all_private, + const char *buf, + const jsmntok_t *toks, + const bool *expose_all_private, const struct short_channel_id *hints, bool *none_public, - bool *deadends, - struct amount_msat *amount_offline); + struct amount_msat *avail_capacity, + struct amount_msat *private_capacity, + struct amount_msat *deadend_capacity, + struct amount_msat *offline_capacity); #endif /* LIGHTNING_LIGHTNINGD_ROUTEHINT_H */ diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index 8a4fd854e..34b7e7e56 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -160,6 +160,12 @@ void connect_succeeded(struct lightningd *ld UNNEEDED, const struct peer *peer U bool incoming UNNEEDED, const struct wireaddr_internal *addr UNNEEDED) { fprintf(stderr, "connect_succeeded called!\n"); abort(); } +/* Generated stub for db_begin_transaction_ */ +void db_begin_transaction_(struct db *db UNNEEDED, const char *location UNNEEDED) +{ fprintf(stderr, "db_begin_transaction_ called!\n"); abort(); } +/* Generated stub for db_commit_transaction */ +void db_commit_transaction(struct db *db UNNEEDED) +{ fprintf(stderr, "db_commit_transaction called!\n"); abort(); } /* Generated stub for delay_then_reconnect */ void delay_then_reconnect(struct channel *channel UNNEEDED, u32 seconds_delay UNNEEDED, const struct wireaddr_internal *addrhint TAKES UNNEEDED) @@ -191,6 +197,10 @@ bool feature_negotiated(const struct feature_set *our_features UNNEEDED, /* Generated stub for feature_offered */ bool feature_offered(const u8 *features UNNEEDED, size_t f UNNEEDED) { fprintf(stderr, "feature_offered called!\n"); abort(); } +/* Generated stub for find_plugin_for_command */ +struct plugin *find_plugin_for_command(struct lightningd *ld UNNEEDED, + const char *cmd_name UNNEEDED) +{ fprintf(stderr, "find_plugin_for_command called!\n"); abort(); } /* Generated stub for fixup_htlcs_out */ void fixup_htlcs_out(struct lightningd *ld UNNEEDED) { fprintf(stderr, "fixup_htlcs_out called!\n"); abort(); } @@ -207,9 +217,6 @@ bool fromwire_channeld_dev_memleak_reply(const void *p UNNEEDED, bool *leak UNNE /* Generated stub for fromwire_connectd_peer_connected */ bool fromwire_connectd_peer_connected(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct node_id *id UNNEEDED, struct wireaddr_internal *addr UNNEEDED, bool *incoming UNNEEDED, struct per_peer_state **pps UNNEEDED, u8 **features UNNEEDED) { fprintf(stderr, "fromwire_connectd_peer_connected called!\n"); abort(); } -/* Generated stub for fromwire_gossipd_get_incoming_channels_reply */ -bool fromwire_gossipd_get_incoming_channels_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct route_info **public_route_info UNNEEDED, bool **public_deadends UNNEEDED, struct route_info **private_route_info UNNEEDED, bool **private_deadends UNNEEDED) -{ fprintf(stderr, "fromwire_gossipd_get_incoming_channels_reply called!\n"); abort(); } /* Generated stub for fromwire_hsmd_sign_bolt12_reply */ bool fromwire_hsmd_sign_bolt12_reply(const void *p UNNEEDED, struct bip340sig *sig UNNEEDED) { fprintf(stderr, "fromwire_hsmd_sign_bolt12_reply called!\n"); abort(); } @@ -389,6 +396,10 @@ enum address_parse_result json_to_address_scriptpubkey(const tal_t *ctx UNNEEDED const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, const u8 **scriptpubkey UNNEEDED) { fprintf(stderr, "json_to_address_scriptpubkey called!\n"); abort(); } +/* Generated stub for json_to_msat */ +bool json_to_msat(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, + struct amount_msat *msat UNNEEDED) +{ fprintf(stderr, "json_to_msat called!\n"); abort(); } /* Generated stub for json_to_node_id */ bool json_to_node_id(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, struct node_id *id UNNEEDED) @@ -405,6 +416,21 @@ bool json_to_short_channel_id(const char *buffer UNNEEDED, const jsmntok_t *tok bool json_tok_channel_id(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, struct channel_id *cid UNNEEDED) { fprintf(stderr, "json_tok_channel_id called!\n"); abort(); } +/* Generated stub for jsonrpc_request_end */ +void jsonrpc_request_end(struct jsonrpc_request *request UNNEEDED) +{ fprintf(stderr, "jsonrpc_request_end called!\n"); abort(); } +/* Generated stub for jsonrpc_request_start_ */ +struct jsonrpc_request *jsonrpc_request_start_( + const tal_t *ctx UNNEEDED, const char *method UNNEEDED, struct log *log UNNEEDED, + void (*notify_cb)(const char *buffer UNNEEDED, + const jsmntok_t *idtok UNNEEDED, + const jsmntok_t *methodtok UNNEEDED, + const jsmntok_t *paramtoks UNNEEDED, + void *) UNNEEDED, + void (*response_cb)(const char *buffer UNNEEDED, const jsmntok_t *toks UNNEEDED, + const jsmntok_t *idtok UNNEEDED, void *) UNNEEDED, + void *response_cb_arg UNNEEDED) +{ fprintf(stderr, "jsonrpc_request_start_ called!\n"); abort(); } /* Generated stub for kill_uncommitted_channel */ void kill_uncommitted_channel(struct uncommitted_channel *uc UNNEEDED, const char *why UNNEEDED) @@ -601,6 +627,10 @@ void per_peer_state_set_fds(struct per_peer_state *pps UNNEEDED, bool plugin_hook_call_(struct lightningd *ld UNNEEDED, const struct plugin_hook *hook UNNEEDED, tal_t *cb_arg STEALS UNNEEDED) { fprintf(stderr, "plugin_hook_call_ called!\n"); abort(); } +/* Generated stub for plugin_request_send */ +void plugin_request_send(struct plugin *plugin UNNEEDED, + struct jsonrpc_request *req TAKES UNNEEDED) +{ fprintf(stderr, "plugin_request_send called!\n"); abort(); } /* Generated stub for subd_req_ */ void subd_req_(const tal_t *ctx UNNEEDED, struct subd *sd UNNEEDED, @@ -647,9 +677,6 @@ u8 *towire_errorfmt(const tal_t *ctx UNNEEDED, const struct channel_id *channel UNNEEDED, const char *fmt UNNEEDED, ...) { fprintf(stderr, "towire_errorfmt called!\n"); abort(); } -/* Generated stub for towire_gossipd_get_incoming_channels */ -u8 *towire_gossipd_get_incoming_channels(const tal_t *ctx UNNEEDED) -{ fprintf(stderr, "towire_gossipd_get_incoming_channels called!\n"); abort(); } /* Generated stub for towire_hsmd_sign_bolt12 */ u8 *towire_hsmd_sign_bolt12(const tal_t *ctx UNNEEDED, const wirestring *messagename UNNEEDED, const wirestring *fieldname UNNEEDED, const struct sha256 *merkleroot UNNEEDED, const u8 *publictweak UNNEEDED) { fprintf(stderr, "towire_hsmd_sign_bolt12 called!\n"); abort(); } diff --git a/tests/test_invoices.py b/tests/test_invoices.py index 995f3c00f..33e91ae9b 100644 --- a/tests/test_invoices.py +++ b/tests/test_invoices.py @@ -160,9 +160,11 @@ def test_invoice_routeboost(node_factory, bitcoind): # Make invoice and pay it inv = l2.rpc.invoice(msatoshi=123456, label="inv1", description="?") # Check routeboost. + assert 'warning_private_unused' not in inv assert 'warning_capacity' not in inv assert 'warning_offline' not in inv assert 'warning_deadends' not in inv + assert 'warning_mpp' not in inv # Route array has single route with single element. r = only_one(only_one(l1.rpc.decodepay(inv['bolt11'])['routes'])) assert r['pubkey'] == l1.info['id'] @@ -178,18 +180,22 @@ def test_invoice_routeboost(node_factory, bitcoind): # Due to reserve & fees, l1 doesn't have capacity to pay this. inv = l2.rpc.invoice(msatoshi=2 * (10**8) - 123456, label="inv2", description="?") # Check warning - assert 'warning_capacity' in inv or 'warning_mpp_capacity' in inv + assert 'warning_capacity' in inv + assert 'warning_private_unused' not in inv assert 'warning_offline' not in inv assert 'warning_deadends' not in inv + assert 'warning_mpp' not in inv l1.rpc.disconnect(l2.info['id'], True) wait_for(lambda: not only_one(l2.rpc.listpeers(l1.info['id'])['peers'])['connected']) inv = l2.rpc.invoice(123456, label="inv3", description="?") # Check warning. + assert 'warning_private_unused' not in inv assert 'warning_capacity' not in inv assert 'warning_deadends' not in inv assert 'warning_offline' in inv + assert 'warning_mpp' not in inv # Close l0, l2 will not use l1 at all. l0.rpc.close(l1.info['id']) @@ -201,8 +207,10 @@ def test_invoice_routeboost(node_factory, bitcoind): inv = l2.rpc.invoice(123456, label="inv4", description="?") # Check warning. assert 'warning_deadends' in inv + assert 'warning_private_unused' not in inv assert 'warning_capacity' not in inv assert 'warning_offline' not in inv + assert 'warning_mpp' not in inv @pytest.mark.developer("gossip without DEVELOPER=1 is slow") @@ -226,9 +234,11 @@ def test_invoice_routeboost_private(node_factory, bitcoind): # Since there's only one route, it will reluctantly hint that even # though it's private inv = l2.rpc.invoice(msatoshi=123456, label="inv0", description="?") + assert 'warning_private_unused' not in inv assert 'warning_capacity' not in inv assert 'warning_offline' not in inv assert 'warning_deadends' not in inv + assert 'warning_mpp' not in inv # Route array has single route with single element. r = only_one(only_one(l1.rpc.decodepay(inv['bolt11'])['routes'])) assert r['pubkey'] == l1.info['id'] @@ -239,16 +249,20 @@ def test_invoice_routeboost_private(node_factory, bitcoind): # If we explicitly say not to, it won't expose. inv = l2.rpc.invoice(msatoshi=123456, label="inv1", description="?", exposeprivatechannels=False) - assert 'warning_capacity' in inv or 'warning_mpp_capacity' in inv + assert 'warning_private_unused' in inv + assert 'warning_capacity' not in inv assert 'warning_offline' not in inv assert 'warning_deadends' not in inv + assert 'warning_mpp' not in inv assert 'routes' not in l1.rpc.decodepay(inv['bolt11']) # If we ask for it, we get it. inv = l2.rpc.invoice(msatoshi=123456, label="inv1a", description="?", exposeprivatechannels=scid) + assert 'warning_private_unused' not in inv assert 'warning_capacity' not in inv assert 'warning_offline' not in inv assert 'warning_deadends' not in inv + assert 'warning_mpp' not in inv # Route array has single route with single element. r = only_one(only_one(l1.rpc.decodepay(inv['bolt11'])['routes'])) assert r['pubkey'] == l1.info['id'] @@ -259,9 +273,11 @@ def test_invoice_routeboost_private(node_factory, bitcoind): # Similarly if we ask for an array. inv = l2.rpc.invoice(msatoshi=123456, label="inv1b", description="?", exposeprivatechannels=[scid]) + assert 'warning_private_unused' not in inv assert 'warning_capacity' not in inv assert 'warning_offline' not in inv assert 'warning_deadends' not in inv + assert 'warning_mpp' not in inv # Route array has single route with single element. r = only_one(only_one(l1.rpc.decodepay(inv['bolt11'])['routes'])) assert r['pubkey'] == l1.info['id'] @@ -280,15 +296,20 @@ def test_invoice_routeboost_private(node_factory, bitcoind): wait_for(lambda: [c['public'] for c in l2.rpc.listchannels(scid2)['channels']] == [True, True]) inv = l2.rpc.invoice(msatoshi=10**7, label="inv2", description="?") + print(inv) assert 'warning_deadends' in inv + assert 'warning_private_unused' not in inv assert 'warning_capacity' not in inv assert 'warning_offline' not in inv + assert 'warning_mpp' not in inv # Unless we tell it to include it. inv = l2.rpc.invoice(msatoshi=10**7, label="inv3", description="?", exposeprivatechannels=True) + assert 'warning_private_unused' not in inv assert 'warning_capacity' not in inv assert 'warning_offline' not in inv assert 'warning_deadends' not in inv + assert 'warning_mpp' not in inv # Route array has single route with single element. r = only_one(only_one(l1.rpc.decodepay(inv['bolt11'])['routes'])) assert r['pubkey'] == l1.info['id'] @@ -298,9 +319,11 @@ def test_invoice_routeboost_private(node_factory, bitcoind): assert r['cltv_expiry_delta'] == 6 inv = l2.rpc.invoice(msatoshi=10**7, label="inv4", description="?", exposeprivatechannels=scid) + assert 'warning_private_unused' not in inv assert 'warning_capacity' not in inv assert 'warning_offline' not in inv assert 'warning_deadends' not in inv + assert 'warning_mpp' not in inv # Route array has single route with single element. r = only_one(only_one(l1.rpc.decodepay(inv['bolt11'])['routes'])) assert r['pubkey'] == l1.info['id'] @@ -311,15 +334,19 @@ def test_invoice_routeboost_private(node_factory, bitcoind): # Ask it explicitly to use a channel it can't (insufficient capacity) inv = l2.rpc.invoice(msatoshi=(10**5) * 1000 + 1, label="inv5", description="?", exposeprivatechannels=scid2) + assert 'warning_private_unused' not in inv assert 'warning_deadends' not in inv - assert 'warning_capacity' in inv or 'warning_mpp_capacity' in inv + assert 'warning_capacity' in inv assert 'warning_offline' not in inv + assert 'warning_mpp' not in inv # Give it two options and it will pick one with suff capacity. inv = l2.rpc.invoice(msatoshi=(10**5) * 1000 + 1, label="inv6", description="?", exposeprivatechannels=[scid2, scid]) + assert 'warning_private_unused' not in inv assert 'warning_capacity' not in inv assert 'warning_offline' not in inv assert 'warning_deadends' not in inv + assert 'warning_mpp' not in inv # Route array has single route with single element. r = only_one(only_one(l1.rpc.decodepay(inv['bolt11'])['routes'])) assert r['pubkey'] == l1.info['id'] @@ -335,9 +362,11 @@ def test_invoice_routeboost_private(node_factory, bitcoind): wait_for(lambda: l2.rpc.listchannels(scid_dummy)['channels'] == []) inv = l2.rpc.invoice(msatoshi=123456, label="inv7", description="?", exposeprivatechannels=scid) + assert 'warning_private_unused' not in inv assert 'warning_capacity' not in inv assert 'warning_offline' not in inv assert 'warning_deadends' not in inv + assert 'warning_mpp' not in inv # Route array has single route with single element. r = only_one(only_one(l1.rpc.decodepay(inv['bolt11'])['routes'])) assert r['pubkey'] == l1.info['id']