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 <rusty@rustcorp.com.au>
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

@@ -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
+\fIwarning_capacity\fR: even using all possible channels, there's not enough incoming capacity to pay this invoice\.
-  the incoming capacity to pay this invoice
 .IP \[bu]
-\fIwarning_capacity\fR if there is no channel that has sufficient
+\fIwarning_offline\fR: there would be enough incoming capacity, but some channels are offline, so there isn't\.
-  incoming capacity
 .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<rusty@rustcorp.com.au\fR> is mainly responsible\.
 
 Main web site: \fIhttps://github.com/ElementsProject/lightning\fR
 
-\" SHA256STAMP:dc375ee583188c669574a8ef203971df650ef221567dccb8df7a81ed5ee4990f
+\" SHA256STAMP:d53ec67cd81a41c7218e282c3d7662933868b25190334e9322de8a90ab99d603

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
+- *warning_capacity*: even using all possible channels, there's not enough incoming capacity to pay this invoice.
-    the incoming capacity to pay this invoice
+- *warning_offline*: there would be enough incoming capacity, but some channels are offline, so there isn't.
-- *warning\_capacity* if there is no channel that has sufficient
+- *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.
-    incoming capacity
+- *warning_private_unused*: there would be enough incoming capacity, but some channels are unannounced and *exposeprivatechannels* is *false*, so there isn't.
-- *warning\_deadends* if there is no channel that is not a dead-end
 - *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
 ------

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,
+					     *candidates)
-					     bool *warning_mpp_capacity)
 {
 	/* 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,43 +653,50 @@ struct invoice_info {
 	struct chanhints *chanhints;
 };
 
-static void gossipd_incoming_channels_reply(struct subd *gossipd,
+/* Add routehints based on listincoming results: NULL means success. */
-					    const u8 *msg,
+static struct command_result *
-					    const int *fs,
+add_routehints(struct invoice_info *info,
-					    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)
 {
-	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;
+	struct amount_msat avail_capacity, deadend_capacity, offline_capacity,
+		private_capacity;
+	struct routehint_candidate *candidates;
+	struct amount_msat total, needed;
 
-	candidates = routehint_candidates(tmpctx, info->cmd->ld, msg,
+	/* Dev code can force routes. */
-					  chanhints ? chanhints->expose_all_private : false,
+	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,
-					  &deadends,
+					  &avail_capacity,
-					  &offline_amt);
+					  &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) {
-		was_pending(command_fail(info->cmd,
+		return command_fail(info->cmd,
 				    INVOICE_HINTS_GAVE_NO_ROUTES,
-					 "None of those hints were suitable local channels"));
+				    "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
@@ -716,28 +719,103 @@ static void gossipd_incoming_channels_reply(struct subd *gossipd,
 						  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,
+						      candidates);
-							      &warning_mpp_capacity);
+		*warning_mpp = (tal_count(info->b11->routes) > 1);
-			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;
+
 	b11enc = bolt11_encode(info, info->b11, false,
 			       hsm_sign_b11, info->cmd->ld);
 
 	/* 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,
+		return command_fail(info->cmd,
 				    INVOICE_PREIMAGE_ALREADY_EXISTS,
-					 "preimage already used"));
+				    "preimage already used");
-		return;
 	}
 
 	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,
+		return command_fail(info->cmd, INVOICE_LABEL_ALREADY_EXISTS,
 				    "Duplicate label '%s'",
-					 info->label->s));
+				    info->label->s);
-		return;
 	}
 
 	/* 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 (warning_no_listincoming)
-	if (tal_count(info->b11->routes) == 0) {
+		json_add_string(response, "warning_listincoming",
-		log_unusual(info->cmd->ld->log,
+				"No listincoming command available, cannot add routehints to invoice");
-			    "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_mpp)
 		json_add_string(response, "warning_mpp",
 				"The invoice might only be payable by MPP-capable payers.");
-	if (warning_mpp_capacity)
+	if (warning_capacity)
-		json_add_string(response, "warning_mpp_capacity",
+		json_add_string(response, "warning_capacity",
-				"The total incoming capacity is still insufficient even if the payer had MPP capability.");
+				"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
+		(*chanhints)->hints = NULL;
-			= tal_arr(*chanhints, struct short_channel_id, 0);
 		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,
+	req = jsonrpc_request_start(info, "listincoming",
-		 take(towire_gossipd_get_incoming_channels(NULL)),
+				    cmd->ld->log,
-		 -1, 0, gossipd_incoming_channels_reply, info);
+				    NULL, listincoming_done,
+				    info);
+	jsonrpc_request_end(req);
 
+	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 = {

lightningd/routehint.c

@@ -1,26 +1,12 @@
 #include <common/bolt11.h>
+#include <common/json_helpers.h>
 #include <common/utils.h>
 #include <gossipd/gossipd_wiregen.h>
+#include <lightningd/lightningd.h>
 #include <lightningd/log.h>
 #include <lightningd/peer_control.h>
 #include <lightningd/routehint.h>
 
-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,
+		     const char *buf,
-		     bool expose_all_private,
+		     const jsmntok_t *toks,
+		     const bool *expose_all_private,
 		     const struct short_channel_id *hints,
 		     bool *none_public,
-		     bool *deadends,
+		     struct amount_msat *avail_capacity,
-		     struct amount_msat *amount_offline)
+		     struct amount_msat *private_capacity,
+		     struct amount_msat *deadend_capacity,
+		     struct amount_msat *offline_capacity)
 {
-	struct routehint_candidate *candidates;
+	struct routehint_candidate *candidates, *privcandidates;
-	struct route_info *inchans, *private;
+	const jsmntok_t *t, *arr;
-	bool *inchan_deadends, *private_deadends;
+	size_t i;
 
-	if (!fromwire_gossipd_get_incoming_channels_reply(tmpctx,
+	log_debug(ld->log, "routehint: %.*s",
-							  incoming_channels_reply,
+		  json_tok_full_len(toks),
-							  &inchans,
+		  json_tok_full(buf, toks));
-							  &inchan_deadends,
-							  &private,
-							  &private_deadends))
-		fatal("Gossip gave bad GOSSIPD_GET_INCOMING_CHANNELS_REPLY %s",
-		      tal_hex(tmpctx, incoming_channels_reply));
 
-	*none_public = (tal_count(inchans) == 0) && (tal_count(private) > 0);
+	/* We get the full JSON, including result. */
-	*deadends = false;
+	t = json_get_member(buf, toks, "result");
-
+	if (!t)
-	/* fromwire explicitly makes empty arrays into NULL */
+		fatal("Missing result from listincoming: %.*s",
-	if (!inchans) {
+		      json_tok_full_len(toks),
-		inchans = tal_arr(tmpctx, struct route_info, 0);
+		      json_tok_full(buf, toks));
-		inchan_deadends = tal_arr(tmpctx, bool, 0);
+	arr = json_get_member(buf, t, "incoming");
-	}
-
-	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);
-		}
-	}
 
 	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++) {
+	/* We combine the JSON output which knows the peers' details,
-		struct peer *peer;
+	 * 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);
+		peer = peer_by_id(ld, &r->pubkey);
-		if (!peer)
+		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)
+		if (!candidate.c) {
-			continue;
+			log_debug(ld->log, "%s: abnormal channel",
-
+				  type_to_string(tmpctx,
-		/* Is it a dead-end? */
+						 struct short_channel_id,
-		if (inchan_deadends[i]) {
+						 &r->short_channel_id));
-			*deadends = true;
 			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,
+			log_debug(ld->log, "%s: offline",
-					     *amount_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];
+
+		/* 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;

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
+ * @buf, @toks: output of listincoming command
- * @expose_all_private: consider private channels too (otherwise iff no public)
+ * @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.
+ * @avail_capacity: total capacity of usable channels.
- * @amount_offline: amount we didn't consider due to offline 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,
+		     const char *buf,
-		     bool expose_all_private,
+		     const jsmntok_t *toks,
+		     const bool *expose_all_private,
 		     const struct short_channel_id *hints,
 		     bool *none_public,
-		     bool *deadends,
+		     struct amount_msat *avail_capacity,
-		     struct amount_msat *amount_offline);
+		     struct amount_msat *private_capacity,
+		     struct amount_msat *deadend_capacity,
+		     struct amount_msat *offline_capacity);
 
 #endif /* LIGHTNING_LIGHTNINGD_ROUTEHINT_H */

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(); }

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']