gossipd: hand all candidates up to lightningd to select routeboost.

This lets us do more flexible filtering in the next patch.  But it also
keeps some weird logic out of gossipd.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

gossipd/gossip_wire.csv

@@ -128,12 +128,15 @@ msgdata,gossip_dev_compact_store_reply,success,bool,
 
 # master -> gossipd: get route_info for our incoming channels
 msgtype,gossip_get_incoming_channels,3025
-msgdata,gossip_get_incoming_channels,private_too,?bool,
 
 # gossipd -> master: here they are.
 msgtype,gossip_get_incoming_channels_reply,3125
-msgdata,gossip_get_incoming_channels_reply,num,u16,
+msgdata,gossip_get_incoming_channels_reply,num_public,u16,
-msgdata,gossip_get_incoming_channels_reply,route_info,route_info,num
+msgdata,gossip_get_incoming_channels_reply,public_route_info,route_info,num_public
+msgdata,gossip_get_incoming_channels_reply,public_deadends,bool,num_public
+msgdata,gossip_get_incoming_channels_reply,num_private,u16,
+msgdata,gossip_get_incoming_channels_reply,private_route_info,route_info,num_private
+msgdata,gossip_get_incoming_channels_reply,private_deadends,bool,num_private
 
 # master -> gossipd: blockheight increased.
 msgtype,gossip_new_blockheight,3026

gossipd/gossipd.c

@@ -1188,18 +1188,6 @@ static bool node_has_public_channels(const struct node *peer,
 	return false;
 }
 
-/*~ The `exposeprivate` flag is a trinary: NULL == dynamic, otherwise
- * value decides.  Thus, we provide two wrappers for clarity: */
-static bool never_expose(bool *exposeprivate)
-{
-	return exposeprivate && !*exposeprivate;
-}
-
-static bool always_expose(bool *exposeprivate)
-{
-	return exposeprivate && *exposeprivate;
-}
-
 /*~ For routeboost, we offer payers a hint of what incoming channels might
  * have capacity for their payment.  To do this, lightningd asks for the
  * information about all channels to this node; but gossipd doesn't know about
@@ -1211,14 +1199,12 @@ static struct io_plan *get_incoming_channels(struct io_conn *conn,
 	struct node *node;
 	struct route_info *public = tal_arr(tmpctx, struct route_info, 0);
 	struct route_info *private = tal_arr(tmpctx, struct route_info, 0);
-	bool has_public;
+	bool *priv_deadends = tal_arr(tmpctx, bool, 0);
-	bool *exposeprivate;
+	bool *pub_deadends = tal_arr(tmpctx, bool, 0);
 
-	if (!fromwire_gossip_get_incoming_channels(tmpctx, msg, &exposeprivate))
+	if (!fromwire_gossip_get_incoming_channels(msg))
 		master_badmsg(WIRE_GOSSIP_GET_INCOMING_CHANNELS, msg);
 
-	has_public = always_expose(exposeprivate);
-
 	node = get_node(daemon->rstate, &daemon->rstate->local_id);
 	if (node) {
 		struct chan_map_iter i;
@@ -1227,6 +1213,7 @@ static struct io_plan *get_incoming_channels(struct io_conn *conn,
 		for (c = first_chan(node, &i); c; c = next_chan(node, &i)) {
 			const struct half_chan *hc;
 			struct route_info ri;
+			bool deadend;
 
 			hc = &c->half[half_chan_to(node, c)];
 
@@ -1239,25 +1226,21 @@ static struct io_plan *get_incoming_channels(struct io_conn *conn,
 			ri.fee_proportional_millionths = hc->proportional_fee;
 			ri.cltv_expiry_delta = hc->delay;
 
-			has_public |= is_chan_public(c);
+			deadend = !node_has_public_channels(other_node(node, c),
-
+							    c);
-			/* If peer doesn't have other public channels,
+			if (is_chan_public(c)) {
-			 * no point giving route */
-			if (!node_has_public_channels(other_node(node, c), c))
-				continue;
-
-			if (always_expose(exposeprivate) || is_chan_public(c))
 				tal_arr_expand(&public, ri);
-			else
+				tal_arr_expand(&pub_deadends, deadend);
+			} else {
 				tal_arr_expand(&private, ri);
+				tal_arr_expand(&priv_deadends, deadend);
+			}
 		}
 	}
 
-	/* If no public channels (even deadend ones!), share private ones. */
+	msg = towire_gossip_get_incoming_channels_reply(NULL,
-	if (!has_public && !never_expose(exposeprivate))
+							public, pub_deadends,
-		msg = towire_gossip_get_incoming_channels_reply(NULL, private);
+							private, priv_deadends);
-	else
-		msg = towire_gossip_get_incoming_channels_reply(NULL, public);
 	daemon_conn_send(daemon->master, take(msg));
 
 	return daemon_conn_read_next(conn, daemon->master);

lightningd/invoice.c

@@ -379,6 +379,7 @@ static struct route_info **select_inchan(const tal_t *ctx,
 					 struct lightningd *ld,
 					 struct amount_msat amount_needed,
 					 const struct route_info *inchans,
+					 const bool *deadends,
 					 bool *any_offline)
 {
 	/* BOLT11 struct wants an array of arrays (can provide multiple routes) */
@@ -413,6 +414,10 @@ static struct route_info **select_inchan(const tal_t *ctx,
 		if (!c)
 			continue;
 
+		/* Is it a dead-end? */
+		if (deadends[i])
+			continue;
+
 		/* Channel balance as seen by our node:
 
 		        |<----------------- capacity ----------------->|
@@ -494,30 +499,88 @@ static struct route_info **select_inchan(const tal_t *ctx,
 }
 
 /* Encapsulating struct while we wait for gossipd to give us incoming channels */
+struct chanhints {
+	bool expose_all_private;
+	struct short_channel_id *hints;
+};
+
 struct invoice_info {
 	struct command *cmd;
 	struct preimage payment_preimage;
 	struct bolt11 *b11;
 	struct json_escape *label;
+	struct chanhints *chanhints;
 };
 
+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 all_true(const bool *barr, size_t n)
+{
+	for (size_t i = 0; i < n; i++) {
+		if (!barr[i])
+			return false;
+	}
+	return true;
+}
+
 static void gossipd_incoming_channels_reply(struct subd *gossipd,
 					    const u8 *msg,
 					    const int *fs,
 					    struct invoice_info *info)
 {
 	struct json_stream *response;
-	struct route_info *inchans;
+	struct route_info *inchans, *private;
+	bool *inchan_deadends, *private_deadends;
 	bool any_offline;
 	struct invoice invoice;
 	char *b11enc;
 	const struct invoice_details *details;
 	struct wallet *wallet = info->cmd->ld->wallet;
+	const struct chanhints *chanhints = info->chanhints;
 
-	if (!fromwire_gossip_get_incoming_channels_reply(tmpctx, msg, &inchans))
+	if (!fromwire_gossip_get_incoming_channels_reply(tmpctx, msg,
+							 &inchans,
+							 &inchan_deadends,
+							 &private,
+							 &private_deadends))
 		fatal("Gossip gave bad GOSSIP_GET_INCOMING_CHANNELS_REPLY %s",
 		      tal_hex(msg, msg));
 
+	/* 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 (chanhints->expose_all_private) {
+		append_routes(&inchans, private);
+		append_bools(&inchan_deadends, private_deadends);
+	} else if (chanhints->hints) {
+		/* FIXME: Implement hint support! */
+		assert(!tal_count(chanhints->hints));
+	} else {
+		/* 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);
+		}
+	}
+
 #if DEVELOPER
 	/* dev-routes overrides this. */
 	any_offline = false;
@@ -528,6 +591,7 @@ static void gossipd_incoming_channels_reply(struct subd *gossipd,
 				info->cmd->ld,
 				info->b11->msat ? *info->b11->msat : AMOUNT_MSAT(1),
 				inchans,
+				inchan_deadends,
 				&any_offline);
 
 	/* FIXME: add private routes if necessary! */
@@ -578,14 +642,19 @@ static void gossipd_incoming_channels_reply(struct subd *gossipd,
 			    any_offline
 			    ? " (among currently connected peers)" : "");
 
-		if (any_offline)
+		if (tal_count(inchans) == 0)
+			json_add_string(response, "warning_capacity",
+					"No channels");
+		else if (all_true(inchan_deadends, tal_count(inchans)))
+			json_add_string(response, "warning_deadends",
+					"No channel with a peer that is not a dead end");
+		else if (any_offline)
 			json_add_string(response, "warning_offline",
 					"No channel with a peer that is currently connected"
 					" has sufficient incoming capacity");
 		else
 			json_add_string(response, "warning_capacity",
-					"No channel with a peer that is not a dead end,"
+					"No channel with a peer that has sufficient incoming capacity");
-					" has sufficient incoming capacity");
 	}
 
 	was_pending(command_success(info->cmd, response));
@@ -739,6 +808,7 @@ static struct command_result *json_invoice(struct command *cmd,
 
 	info = tal(cmd, struct invoice_info);
 	info->cmd = cmd;
+	info->chanhints = tal(info, struct chanhints);
 
 	if (!param(cmd, buffer, params,
 		   p_req("msatoshi", param_msat_or_any, &msatoshi_val),
@@ -769,6 +839,17 @@ static struct command_result *json_invoice(struct command *cmd,
 				    strlen(desc_val));
 	}
 
+	/* Default is expose iff no public channels. */
+	if (exposeprivate == NULL) {
+		info->chanhints->expose_all_private = false;
+		info->chanhints->hints = NULL;
+	} else {
+		info->chanhints->expose_all_private = *exposeprivate;
+		/* FIXME: Support hints! */
+		info->chanhints->hints = tal_arr(info->chanhints,
+						 struct short_channel_id, 0);
+	}
+
 	if (msatoshi_val
 	    && amount_msat_greater(*msatoshi_val, chainparams->max_payment)) {
 		return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
@@ -829,10 +910,8 @@ static struct command_result *json_invoice(struct command *cmd,
 	if (fallback_scripts)
 		info->b11->fallbacks = tal_steal(info->b11, fallback_scripts);
 
-	log_debug(cmd->ld->log, "exposeprivate = %s",
-		  exposeprivate ? (*exposeprivate ? "TRUE" : "FALSE") : "NULL");
 	subd_req(cmd, cmd->ld->gossip,
-		 take(towire_gossip_get_incoming_channels(NULL, exposeprivate)),
+		 take(towire_gossip_get_incoming_channels(NULL)),
 		 -1, 0, gossipd_incoming_channels_reply, info);
 
 	return command_still_pending(cmd);

lightningd/test/run-invoice-select-inchan.c

@@ -107,7 +107,7 @@ bool fromwire_channel_dev_memleak_reply(const void *p UNNEEDED, bool *leak UNNEE
 bool fromwire_connect_peer_connected(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct node_id *id UNNEEDED, struct wireaddr_internal *addr UNNEEDED, struct per_peer_state **pps UNNEEDED, u8 **features UNNEEDED)
 { fprintf(stderr, "fromwire_connect_peer_connected called!\n"); abort(); }
 /* Generated stub for fromwire_gossip_get_incoming_channels_reply */
-bool fromwire_gossip_get_incoming_channels_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct route_info **route_info UNNEEDED)
+bool fromwire_gossip_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_gossip_get_incoming_channels_reply called!\n"); abort(); }
 /* Generated stub for fromwire_hsm_get_channel_basepoints_reply */
 bool fromwire_hsm_get_channel_basepoints_reply(const void *p UNNEEDED, struct basepoints *basepoints UNNEEDED, struct pubkey *funding_pubkey UNNEEDED)
@@ -473,7 +473,7 @@ u8 *towire_errorfmt(const tal_t *ctx UNNEEDED,
 		    const char *fmt UNNEEDED, ...)
 { fprintf(stderr, "towire_errorfmt called!\n"); abort(); }
 /* Generated stub for towire_gossip_get_incoming_channels */
-u8 *towire_gossip_get_incoming_channels(const tal_t *ctx UNNEEDED, bool *private_too UNNEEDED)
+u8 *towire_gossip_get_incoming_channels(const tal_t *ctx UNNEEDED)
 { fprintf(stderr, "towire_gossip_get_incoming_channels called!\n"); abort(); }
 /* Generated stub for towire_hsm_get_channel_basepoints */
 u8 *towire_hsm_get_channel_basepoints(const tal_t *ctx UNNEEDED, const struct node_id *peerid UNNEEDED, u64 dbid UNNEEDED)
@@ -686,6 +686,7 @@ int main(void)
 	struct lightningd *ld;
 	bool any_offline;
 	struct route_info *inchans;
+	bool *deadends;
 	struct route_info **ret;
 	size_t n;
 
@@ -698,51 +699,53 @@ int main(void)
 	list_head_init(&ld->peers);
 
 	inchans = tal_arr(tmpctx, struct route_info, 0);
+	deadends = tal_arrz(tmpctx, bool, 100);
+
 	/* 1. Nothing to choose from -> NULL result. */
-	assert(select_inchan(tmpctx, ld, AMOUNT_MSAT(0), inchans, &any_offline) == NULL);
+	assert(select_inchan(tmpctx, ld, AMOUNT_MSAT(0), inchans, deadends, &any_offline) == NULL);
 	assert(any_offline == false);
 
 	/* 2. inchan but no corresponding peer -> NULL result. */
 	add_inchan(&inchans, 0);
-	assert(select_inchan(tmpctx, ld, AMOUNT_MSAT(0), inchans, &any_offline) == NULL);
+	assert(select_inchan(tmpctx, ld, AMOUNT_MSAT(0), inchans, deadends, &any_offline) == NULL);
 	assert(any_offline == false);
 
 	/* 3. inchan but its peer in awaiting lockin -> NULL result. */
 	add_peer(ld, 0, CHANNELD_AWAITING_LOCKIN, true);
-	assert(select_inchan(tmpctx, ld, AMOUNT_MSAT(0), inchans, &any_offline) == NULL);
+	assert(select_inchan(tmpctx, ld, AMOUNT_MSAT(0), inchans, deadends, &any_offline) == NULL);
 	assert(any_offline == false);
 
 	/* 4. connected peer but no corresponding inchan -> NULL result. */
 	add_peer(ld, 1, CHANNELD_NORMAL, true);
-	assert(select_inchan(tmpctx, ld, AMOUNT_MSAT(0), inchans, &any_offline) == NULL);
+	assert(select_inchan(tmpctx, ld, AMOUNT_MSAT(0), inchans, deadends, &any_offline) == NULL);
 	assert(any_offline == false);
 
 	/* 5. inchan but its peer (replaced with one) offline -> NULL result. */
 	list_del_from(&ld->peers, &list_tail(&ld->peers, struct peer, list)->list);
 	add_peer(ld, 1, CHANNELD_NORMAL, false);
 	add_inchan(&inchans, 1);
-	assert(select_inchan(tmpctx, ld, AMOUNT_MSAT(0), inchans, &any_offline) == NULL);
+	assert(select_inchan(tmpctx, ld, AMOUNT_MSAT(0), inchans, deadends, &any_offline) == NULL);
 	assert(any_offline == true);
 
 	/* 6. Finally, a correct peer! */
 	add_inchan(&inchans, 2);
 	add_peer(ld, 2, CHANNELD_NORMAL, true);
 
-	ret = select_inchan(tmpctx, ld, AMOUNT_MSAT(0), inchans, &any_offline);
+	ret = select_inchan(tmpctx, ld, AMOUNT_MSAT(0), inchans, deadends, &any_offline);
 	assert(tal_count(ret) == 1);
 	assert(tal_count(ret[0]) == 1);
 	assert(any_offline == true); /* Peer 1 is offline */
 	assert(route_info_eq(ret[0], &inchans[2]));
 
 	/* 7. Correct peer with just enough capacity_to_pay_us */
-	ret = select_inchan(tmpctx, ld, AMOUNT_MSAT(1999), inchans, &any_offline);
+	ret = select_inchan(tmpctx, ld, AMOUNT_MSAT(1999), inchans, deadends, &any_offline);
 	assert(tal_count(ret) == 1);
 	assert(tal_count(ret[0]) == 1);
 	assert(any_offline == false); /* Other candidate insufficient funds. */
 	assert(route_info_eq(ret[0], &inchans[2]));
 
 	/* 8. Not if we ask for too much! Our balance is 1msat. */
-	ret = select_inchan(tmpctx, ld, AMOUNT_MSAT(2000), inchans, &any_offline);
+	ret = select_inchan(tmpctx, ld, AMOUNT_MSAT(2000), inchans, deadends, &any_offline);
 	assert(ret == NULL);
 	assert(any_offline == false); /* Other candidate insufficient funds. */
 
@@ -752,7 +755,7 @@ int main(void)
 
 	/* Simulate selection ratios between excesses 25% and 50% of capacity*/
 	for (size_t i = n = 0; i < 1000; i++) {
-		ret = select_inchan(tmpctx, ld, AMOUNT_MSAT(1499), inchans, &any_offline);
+		ret = select_inchan(tmpctx, ld, AMOUNT_MSAT(1499), inchans, deadends, &any_offline);
 		assert(tal_count(ret) == 1);
 		assert(tal_count(ret[0]) == 1);
 		assert(any_offline == false); /* Other candidate insufficient funds. */
@@ -770,11 +773,11 @@ int main(void)
 	/* 10. Last peer's capacity goes from 3 to 2 sat*/
 		list_tail(&list_tail(&ld->peers, struct peer, list)->channels, struct channel, list)->
 				channel_info.their_config.channel_reserve = AMOUNT_SAT(1);
-		ret = select_inchan(tmpctx, ld, AMOUNT_MSAT(1499), inchans, &any_offline);
+		ret = select_inchan(tmpctx, ld, AMOUNT_MSAT(1499), inchans, deadends, &any_offline);
 
 		/* Simulate selection ratios between excesses 25% and 75% of capacity*/
 	for (size_t i = n = 0; i < 1000; i++) {
-		ret = select_inchan(tmpctx, ld, AMOUNT_MSAT(1499), inchans, &any_offline);
+		ret = select_inchan(tmpctx, ld, AMOUNT_MSAT(1499), inchans, deadends, &any_offline);
 		assert(tal_count(ret) == 1);
 		assert(tal_count(ret[0]) == 1);
 		assert(any_offline == false); /* Other candidate insufficient funds. */

tests/test_invoices.py

@@ -136,6 +136,7 @@ def test_invoice_routeboost(node_factory, bitcoind):
     # Check routeboost.
     assert 'warning_capacity' not in inv
     assert 'warning_offline' not in inv
+    assert 'warning_deadends' 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']
@@ -153,6 +154,7 @@ def test_invoice_routeboost(node_factory, bitcoind):
     # Check warning
     assert 'warning_capacity' in inv
     assert 'warning_offline' not in inv
+    assert 'warning_deadends' not in inv
 
     l1.rpc.disconnect(l2.info['id'], True)
     wait_for(lambda: not only_one(l2.rpc.listpeers(l1.info['id'])['peers'])['connected'])
@@ -160,6 +162,7 @@ def test_invoice_routeboost(node_factory, bitcoind):
     inv = l2.rpc.invoice(123456, label="inv3", description="?")
     # Check warning.
     assert 'warning_capacity' not in inv
+    assert 'warning_deadends' not in inv
     assert 'warning_offline' in inv
 
     # Close l0, l2 will not use l1 at all.
@@ -171,7 +174,8 @@ def test_invoice_routeboost(node_factory, bitcoind):
     wait_for(lambda: len(l2.rpc.listchannels()['channels']) == 2)
     inv = l2.rpc.invoice(123456, label="inv4", description="?")
     # Check warning.
-    assert 'warning_capacity' in inv
+    assert 'warning_deadends' in inv
+    assert 'warning_capacity' not in inv
     assert 'warning_offline' not in inv
 
 
@@ -195,6 +199,7 @@ def test_invoice_routeboost_private(node_factory, bitcoind):
     inv = l2.rpc.invoice(msatoshi=123456, label="inv0", description="?")
     assert 'warning_capacity' not in inv
     assert 'warning_offline' not in inv
+    assert 'warning_deadends' 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']
@@ -206,6 +211,8 @@ 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
+    assert 'warning_offline' not in inv
+    assert 'warning_deadends' not in inv
     assert 'routes' not in l1.rpc.decodepay(inv['bolt11'])
 
     # The existence of a public channel, even without capacity, will suppress
@@ -219,12 +226,15 @@ def test_invoice_routeboost_private(node_factory, bitcoind):
     wait_for(lambda: [c['public'] for c in l3.rpc.listchannels(scid)['channels']] == [True, True])
 
     inv = l2.rpc.invoice(msatoshi=10**7, label="inv2", description="?")
-    assert 'warning_capacity' in inv
+    assert 'warning_deadends' in inv
+    assert 'warning_capacity' not in inv
+    assert 'warning_offline' not in inv
 
     # Unless we tell it to include it.
     inv = l2.rpc.invoice(msatoshi=10**7, label="inv3", description="?", exposeprivatechannels=True)
     assert 'warning_capacity' not in inv
     assert 'warning_offline' not in inv
+    assert 'warning_deadends' 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']