diff --git a/gossipd/gossip_wire.csv b/gossipd/gossip_wire.csv index a1674d434..c7190e37c 100644 --- a/gossipd/gossip_wire.csv +++ b/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,route_info,route_info,num +msgdata,gossip_get_incoming_channels_reply,num_public,u16, +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 diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index 3c8d8303a..1abbecf9a 100644 --- a/gossipd/gossipd.c +++ b/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 *exposeprivate; + bool *priv_deadends = tal_arr(tmpctx, bool, 0); + 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); - - /* If peer doesn't have other public channels, - * no point giving route */ - if (!node_has_public_channels(other_node(node, c), c)) - continue; - - if (always_expose(exposeprivate) || is_chan_public(c)) + deadend = !node_has_public_channels(other_node(node, c), + c); + if (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. */ - if (!has_public && !never_expose(exposeprivate)) - msg = towire_gossip_get_incoming_channels_reply(NULL, private); - else - msg = towire_gossip_get_incoming_channels_reply(NULL, public); + msg = towire_gossip_get_incoming_channels_reply(NULL, + public, pub_deadends, + private, priv_deadends); daemon_conn_send(daemon->master, take(msg)); return daemon_conn_read_next(conn, daemon->master); diff --git a/lightningd/invoice.c b/lightningd/invoice.c index 114900379..c8e9ef4ad 100644 --- a/lightningd/invoice.c +++ b/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," - " has sufficient incoming capacity"); + "No channel with a peer that 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); diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index d4bd272bc..f49a19f30 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/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. */ diff --git a/tests/test_invoices.py b/tests/test_invoices.py index f64c40deb..4393eb81e 100644 --- a/tests/test_invoices.py +++ b/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']