Skip to content

Commit

Permalink
lightningd: fix/refactor select_inchan for invoice route-hint, use fr…
Browse files Browse the repository at this point in the history 
…actional excess as weight

Refactored the weighted-reservoir-sampling algo to make it more straightforward.
It now uses the excess as fraction of capacity as weight. This favors channels that
are more _relatively_ unbalanced to be used for incoming payment.

Now passes test_invoice_routeboost_private() when using max fundamount=16777215.
  • Loading branch information
@SimonVrouwe @rustyrussell
SimonVrouwe authored and rustyrussell committed Apr 16, 2019
1 parent 06a062f commit c053dc9
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 44 deletions.
108 changes: 83 additions & 25 deletions lightningd/invoice.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,27 +313,45 @@ static struct command_result *parse_fallback(struct command *cmd,
return NULL;
}

/* BOLT11 struct wants an array of arrays (can provide multiple routes) */
/*
* From array of incoming channels [inchan], find suitable ones for
* a payment-to-us of [amount_needed], using criteria:
* 1. Channel's peer is known, in state CHANNELD_NORMAL and is online.
* 2. Channel's peer capacity to pay us is sufficient.
*
* Then use weighted reservoir sampling, which makes probing channel balances
* harder, to choose one channel from the set of suitable channels. It favors
* channels that have less balance on our side as fraction of their capacity.
*
* [any_offline] is set if the peer of any suitable channel appears offline.
*/
static struct route_info **select_inchan(const tal_t *ctx,
struct lightningd *ld,
struct amount_msat capacity_needed,
struct amount_msat amount_needed,
const struct route_info *inchans,
bool *any_offline)
{
const struct route_info *r = NULL;
struct route_info **ret;
/* BOLT11 struct wants an array of arrays (can provide multiple routes) */
struct route_info **R;
double wsum, p;

struct sample {
const struct route_info *route;
double weight;
};

struct sample *S = tal_arr(tmpctx, struct sample, 0);

*any_offline = false;

/* Weighted reservoir sampling.
* Based on https://en.wikipedia.org/wiki/Reservoir_sampling
* Algorithm A-Chao
*/
u64 wsum = 0;
/* Collect suitable channels and assign each a weight. */
for (size_t i = 0; i < tal_count(inchans); i++) {
struct peer *peer;
struct channel *c;
struct amount_msat avail, excess;
struct sample sample;
struct amount_msat their_msat, capacity_to_pay_us, excess, capacity;
struct amount_sat cumulative_reserve;
double excess_frac;

/* Do we know about this peer? */
peer = peer_by_id(ld, &inchans[i].pubkey);
Expand All @@ -345,8 +363,23 @@ static struct route_info **select_inchan(const tal_t *ctx,
if (!c)
continue;

/* Does it have sufficient capacity. */
if (!amount_sat_sub_msat(&avail, c->funding, c->our_msat)) {
/* Channel balance as seen by our node:
|<----------------- capacity ----------------->|
. .
. |<------------------ their_msat -------------------->|
. | . |
. |<----- capacity_to_pay_us ----->|<- their_reserve ->|
. | | |
. |<- amount_needed --><- excess ->| |
. | | |
|-------|-------------|--------------------------------|-------------------|
0 ^ ^ ^ funding
our_reserve our_msat */

/* Does the peer have sufficient balance to pay us. */
if (!amount_sat_sub_msat(&their_msat, c->funding, c->our_msat)) {

log_broken(ld->log,
"underflow: funding %s - our_msat %s",
type_to_string(tmpctx, struct amount_sat,
Expand All @@ -356,12 +389,12 @@ static struct route_info **select_inchan(const tal_t *ctx,
continue;
}

/* Even after reserve taken into account */
if (!amount_msat_sub_sat(&avail,
avail, c->our_config.channel_reserve))
/* Even after taken into account their reserve */
if (!amount_msat_sub_sat(&capacity_to_pay_us, their_msat,
c->our_config.channel_reserve))
continue;

if (!amount_msat_sub(&excess, avail, capacity_needed))
if (!amount_msat_sub(&excess, capacity_to_pay_us, amount_needed))
continue;

/* Is it offline? */
Expand All @@ -370,19 +403,44 @@ static struct route_info **select_inchan(const tal_t *ctx,
continue;
}

/* Avoid divide-by-zero corner case. */
wsum += excess.millisatoshis + 1; /* Raw: rand select */
if (pseudorand(1ULL << 32)
<= ((excess.millisatoshis + 1) << 32) / wsum) /* Raw: rand select */
r = &inchans[i];
/* Find capacity and calculate its excess fraction */
if (!amount_sat_add(&cumulative_reserve,
c->our_config.channel_reserve,
c->channel_info.their_config.channel_reserve)
|| !amount_sat_to_msat(&capacity, c->funding)
|| !amount_msat_sub_sat(&capacity, capacity, cumulative_reserve)) {
log_broken(ld->log, "Channel %s capacity overflow!",
type_to_string(tmpctx, struct short_channel_id, c->scid));
continue;
}

excess_frac = (double)excess.millisatoshis / capacity.millisatoshis; /* Raw: double fraction */

sample.route = &inchans[i];
sample.weight = excess_frac;
tal_arr_expand(&S, sample);
}

if (!r)
if (!tal_count(S))
return NULL;

ret = tal_arr(ctx, struct route_info *, 1);
ret[0] = tal_dup(ret, struct route_info, r);
return ret;
/* Use weighted reservoir sampling, see:
* https://en.wikipedia.org/wiki/Reservoir_sampling#Algorithm_A-Chao
* But (currently) the result will consist of only one sample (k=1) */
R = tal_arr(ctx, struct route_info *, 1);
R[0] = tal_dup(R, struct route_info, S[0].route);
wsum = S[0].weight;

for (size_t i = 1; i < tal_count(S); i++) {
wsum += S[i].weight;
p = S[i].weight / wsum;
double random_1 = pseudorand_double(); /* range [0,1) */

if (random_1 <= p)
R[0] = tal_dup(R, struct route_info, S[i].route);
}

return R;
}

/* Encapsulating struct while we wait for gossipd to give us incoming channels */
Expand Down
48 changes: 30 additions & 18 deletions lightningd/test/run-invoice-select-inchan.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,7 @@ static void add_peer(struct lightningd *ld, int n, enum channel_state state,
c->funding.satoshis = n+1;
c->our_msat = AMOUNT_MSAT(1);
c->our_config.channel_reserve = AMOUNT_SAT(1);
c->channel_info.their_config.channel_reserve = AMOUNT_SAT(0);
list_add_tail(&peer->channels, &c->list);
}

Expand Down Expand Up @@ -647,57 +648,61 @@ int main(void)
list_head_init(&ld->peers);

inchans = tal_arr(tmpctx, struct route_info, 0);
/* Nothing to choose from -> NULL result. */
/* 1. Nothing to choose from -> NULL result. */
assert(select_inchan(tmpctx, ld, AMOUNT_MSAT(0), inchans, &any_offline) == NULL);
assert(any_offline == false);

/* inchan but no peer -> NULL result. */
/* 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(any_offline == false);

/* connected peer but no inchan -> NULL result. */
add_peer(ld, 1, CHANNELD_NORMAL, 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(any_offline == false);

/* inchan but peer awaiting lockin -> NULL result. */
add_peer(ld, 0, CHANNELD_AWAITING_LOCKIN, true);
/* 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(any_offline == false);

/* inchan but peer not connected -> NULL result. */
/* 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(any_offline == true);

/* Finally, a correct peer! */
/* 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);
assert(tal_count(ret) == 1);
assert(tal_count(ret[0]) == 1);
assert(any_offline == true);
assert(any_offline == true); /* Peer 1 is offline */
assert(route_info_eq(ret[0], &inchans[2]));

/* Not if we ask for too much! Reserve is 1 satoshi */
/* 7. Correct peer with just enough capacity_to_pay_us */
ret = select_inchan(tmpctx, ld, AMOUNT_MSAT(1999), inchans, &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);
assert(ret == NULL);
assert(any_offline == false); /* Other candidate insufficient funds. */

/* Add another candidate, with twice as much excess. */
/* 9. Add another peer */
add_inchan(&inchans, 3);
add_peer(ld, 3, CHANNELD_NORMAL, true);

/* 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(1000), inchans, &any_offline);
ret = select_inchan(tmpctx, ld, AMOUNT_MSAT(1499), inchans, &any_offline);
assert(tal_count(ret) == 1);
assert(tal_count(ret[0]) == 1);
assert(any_offline == false); /* Other candidate insufficient funds. */
Expand All @@ -707,11 +712,17 @@ int main(void)
}

/* Handwave over probability of this happening! Within 20% */
assert(n > 333 - 66 && n < 333 + 66);
printf("Number of selections with 999 excess: %zu\n"
"Number of selections with 1999 excess: %zu\n",
printf("Number of selections with excess 25 percent of capacity: %zu\n"
"Number of selections with excess 50 percent of capacity: %zu\n",
n, 1000 - n);
assert(n > 333 - 66 && n < 333 + 66);

/* 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);

/* 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);
assert(tal_count(ret) == 1);
Expand All @@ -722,10 +733,11 @@ int main(void)
n += route_info_eq(ret[0], &inchans[2]);
}

assert(n > 250 - 50 && n < 250 + 50);
printf("Number of selections with 500 excess: %zu\n"
"Number of selections with 1500 excess: %zu\n",
/* Handwave over probability of this happening! Within 20% */
printf("Number of selections with excess 25 percent of capacity: %zu\n"
"Number of selections with excess 75 percent of capacity: %zu\n",
n, 1000 - n);
assert(n > 250 - 50 && n < 250 + 50);

/* No memory leaks please */
secp256k1_context_destroy(secp256k1_ctx);
Expand Down
2 changes: 1 addition & 1 deletion tests/test_invoices.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ def test_invoice_routeboost(node_factory, bitcoind):
def test_invoice_routeboost_private(node_factory, bitcoind):
"""Test routeboost 'r' hint in bolt11 invoice for private channels
"""
l1, l2 = node_factory.line_graph(2, fundamount=10**6, announce_channels=False)
l1, l2 = node_factory.line_graph(2, fundamount=16777215, announce_channels=False)

# Attach public channel to l1 so it doesn't look like a dead-end.
l0 = node_factory.get_node()
Expand Down

0 comments on commit c053dc9

Please sign in to comment.