diff --git a/CHANGELOG.md b/CHANGELOG.md index d51ca91bd68d..cf44d1acedd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,11 +11,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - plugins: fully enabled, and ready for you to write some! - plugins: `pay` is now a plugin. +- protocol: `pay` will now use routehints in invoices if it needs to. - lightning-cli: `help ` finds man pages even if `make install` not run. - JSON API: `waitsendpay` now has an `erring_direction` field. - JSON API: `listpeers` now has a `direction` field in `channels`. - JSON API: `listchannels` now takes a `source` option to filter by node id. - JSON API: New command `paystatus` gives detailed information on `pay` commands. +- JSON API: `getroute` `riskfactor` argument is simplified; `pay` now defaults to setting it to 10. ### Changed @@ -38,6 +40,8 @@ changes. (eg. 4 billion) was slow due to a bug. - Fixed occasional deadlock with peers when exchanging huge amounts of gossip. - You can no longer make giant unpayable "wumbo" invoices. +- CLTV of total route now correctly evaluated when finding best route. +- `riskfactor` arguments to `pay` and `getroute` now have an effect. ### Security diff --git a/common/pseudorand.c b/common/pseudorand.c index dc8f6b17b567..817a9e5bfa9a 100644 --- a/common/pseudorand.c +++ b/common/pseudorand.c @@ -37,6 +37,13 @@ uint64_t pseudorand(uint64_t max) return isaac64_next_uint(&isaac64, max); } +uint64_t pseudorand_u64(void) +{ + init_if_needed(); + + return isaac64_next_uint64(&isaac64); +} + const struct siphash_seed *siphash_seed(void) { init_if_needed(); diff --git a/common/pseudorand.h b/common/pseudorand.h index d66435ed44d5..0a46223bffb2 100644 --- a/common/pseudorand.h +++ b/common/pseudorand.h @@ -8,6 +8,11 @@ */ uint64_t pseudorand(uint64_t max); +/** + * pseudorand - pseudo (guessable!) random number between 0 and UINT64_MAX. + */ +uint64_t pseudorand_u64(void); + /** * Get the siphash seed for hash tables. */ diff --git a/doc/lightning-getroute.7 b/doc/lightning-getroute.7 index af94a0aa3e88..3d9de867557f 100644 --- a/doc/lightning-getroute.7 +++ b/doc/lightning-getroute.7 @@ -2,12 +2,12 @@ .\" Title: lightning-getroute .\" Author: [see the "AUTHOR" section] .\" Generator: DocBook XSL Stylesheets v1.79.1 -.\" Date: 01/23/2019 +.\" Date: 02/01/2019 .\" Manual: \ \& .\" Source: \ \& .\" Language: English .\" -.TH "LIGHTNING\-GETROUTE" "7" "01/23/2019" "\ \&" "\ \&" +.TH "LIGHTNING\-GETROUTE" "7" "02/01/2019" "\ \&" "\ \&" .\" ----------------------------------------------------------------- .\" * Define some portability stuff .\" ----------------------------------------------------------------- @@ -36,9 +36,9 @@ lightning-getroute \- Command for routing a payment (low\-level)\&. .sp The \fBgetroute\fR RPC command attempts to find the best route for the payment of \fImsatoshi\fR to lightning node \fIid\fR, such that the payment will arrive at \fIid\fR with \fIcltv\fR\-blocks to spare (default 9)\&. .sp -There are two considerations for how good a route is: how low the fees are, and how long your payment will get stuck if a node goes down during the process\&. The \fIriskfactor\fR floating\-point field controls this tradeoff; it is the annual cost of your funds being stuck (as a percentage), multiplied by the percentage chance of each node failing\&. +There are two considerations for how good a route is: how low the fees are, and how long your payment will get stuck if a node goes down during the process\&. The \fIriskfactor\fR floating\-point field controls this tradeoff; it is the annual cost of your funds being stuck (as a percentage)\&. .sp -For example, if you thought there was a 1% chance that a node would fail, and it would cost you 20% per annum if that happened, \fIriskfactor\fR would be 20\&. +For example, if you thought the inconvenience of having funds stuck was worth 20% per annum interest, \fIriskfactor\fR would be 20\&. .sp If you didn\(cqt care about risk, \fIriskfactor\fR would be zero\&. .sp @@ -55,15 +55,13 @@ The formula used is the following approximation: .RS 4 .\} .nf -hop\-risk = num\-hops x per\-hop\-risk -timeout\-cost = blocks\-timeout x per\-block\-cost -risk\-fee = amount x hop\-risk x timeout\-cost +risk\-fee = amount x blocks\-timeout x per\-block\-cost .fi .if n \{\ .RE .\} .sp -We are given a \fIriskfactor\fR; expressed as two multiplied percentages is the same as fractions multiplied by 10000\&. There are 52596 blocks per year, thus \fIper\-block\-cost\fR x \fIper\-hop\-risk\fR is riskfactor\*(Aq divided by 5,259,600,000\&. +We are given a \fIriskfactor\fR expressed as a percentage\&. There are 52596 blocks per year, thus \fIper\-block\-cost\fR is \fIriskfactor\fR divided by 5,259,600\&. .sp The final result is: .sp @@ -71,26 +69,26 @@ The final result is: .RS 4 .\} .nf -risk\-fee = amount x num\-hops x blocks\-timeout x riskfactor / 5259600000 +risk\-fee = amount x blocks\-timeout x riskfactor / 5259600 .fi .if n \{\ .RE .\} .sp -Here are the risk fees as a percentage of the amount sent, using various parameters\&. For comparison with actual fees, we assume nodes charge 0\&.05%: +Here are the risk fees in millisatoshis, using various parameters\&. I assume a channel charges the default of 1000 millisatoshis plus 1 part\-per\-million\&. Common delay values on the network at 14 and 144\&. .TS allbox tab(:); ltB ltB ltB ltB ltB. T{ -Riskfactor +Amount (msat) T}:T{ -Nodes +Riskfactor T}:T{ -Delay per node +Delay T}:T{ -Risk Fee % +Risk Fee T}:T{ -Route fee % +Route fee T} .T& lt lt lt lt lt @@ -101,160 +99,417 @@ lt lt lt lt lt lt lt lt lt lt lt lt lt lt lt lt lt lt lt lt +lt lt lt lt lt +lt lt lt lt lt +lt lt lt lt lt +lt lt lt lt lt +lt lt lt lt lt +lt lt lt lt lt +lt lt lt lt lt +lt lt lt lt lt +lt lt lt lt lt +lt lt lt lt lt +lt lt lt lt lt +lt lt lt lt lt +lt lt lt lt lt +lt lt lt lt lt +lt lt lt lt lt lt lt lt lt lt. T{ .sp -0\&.001 +10,000 T}:T{ .sp -5 +1 T}:T{ .sp -6 +14 T}:T{ .sp 0 T}:T{ .sp -0\&.25 +1001 T} T{ .sp -1 +10,000 T}:T{ .sp -5 +10 T}:T{ .sp -6 +14 T}:T{ .sp 0 T}:T{ .sp -0\&.25 +1001 +T} +T{ +.sp +10,000 +T}:T{ +.sp +100 +T}:T{ +.sp +14 +T}:T{ +.sp +2 +T}:T{ +.sp +1001 T} T{ .sp +10,000 +T}:T{ +.sp 1000 T}:T{ .sp -5 +14 T}:T{ .sp -6 +26 T}:T{ .sp -0\&.0029 +1001 +T} +T{ +.sp +1,000,000 T}:T{ .sp -0\&.25 +1 +T}:T{ +.sp +14 +T}:T{ +.sp +2 +T}:T{ +.sp +1001 T} T{ .sp -0\&.001 +1,000,000 T}:T{ .sp 10 T}:T{ .sp -72 +14 T}:T{ .sp -0 +26 T}:T{ .sp -0\&.5 +1001 T} T{ .sp +1,000,000 +T}:T{ +.sp +100 +T}:T{ +.sp +14 +T}:T{ +.sp +266 +T}:T{ +.sp +1001 +T} +T{ +.sp +1,000,000 +T}:T{ +.sp +1000 +T}:T{ +.sp +14 +T}:T{ +.sp +2661 +T}:T{ +.sp +1001 +T} +T{ +.sp +100,000,000 +T}:T{ +.sp 1 T}:T{ .sp +14 +T}:T{ +.sp +266 +T}:T{ +.sp +1100 +T} +T{ +.sp +100,000,000 +T}:T{ +.sp 10 T}:T{ .sp -72 +14 +T}:T{ +.sp +2661 +T}:T{ +.sp +1100 +T} +T{ +.sp +100,000,000 +T}:T{ +.sp +100 T}:T{ .sp -0\&.0001 +14 T}:T{ .sp -0\&.5 +26617 +T}:T{ +.sp +1100 T} T{ .sp +100,000,000 +T}:T{ +.sp 1000 T}:T{ .sp +14 +T}:T{ +.sp +266179 +T}:T{ +.sp +1100 +T} +T{ +.sp +10,000 +T}:T{ +.sp +1 +T}:T{ +.sp +144 +T}:T{ +.sp +0 +T}:T{ +.sp +1001 +T} +T{ +.sp +10,000 +T}:T{ +.sp 10 T}:T{ .sp -72 +144 T}:T{ .sp -0\&.1369 +2 T}:T{ .sp -0\&.5 +1001 T} T{ .sp -0\&.001 +10,000 T}:T{ .sp -20 +100 T}:T{ .sp -1008 +144 T}:T{ .sp -0 +27 T}:T{ .sp -1\&.0 +1001 T} T{ .sp +10,000 +T}:T{ +.sp +1000 +T}:T{ +.sp +144 +T}:T{ +.sp +273 +T}:T{ +.sp +1001 +T} +T{ +.sp +1,000,000 +T}:T{ +.sp 1 T}:T{ .sp -20 +144 +T}:T{ +.sp +27 +T}:T{ +.sp +1001 +T} +T{ +.sp +1,000,000 +T}:T{ +.sp +10 +T}:T{ +.sp +144 +T}:T{ +.sp +273 +T}:T{ +.sp +1001 +T} +T{ +.sp +1,000,000 +T}:T{ +.sp +100 T}:T{ .sp -1008 +144 T}:T{ .sp -0\&.0077 +2737 T}:T{ .sp -1\&.0 +1001 T} T{ .sp +1,000,000 +T}:T{ +.sp 1000 T}:T{ .sp -20 +144 +T}:T{ +.sp +27378 +T}:T{ +.sp +1001 +T} +T{ +.sp +100,000,000 +T}:T{ +.sp +1 +T}:T{ +.sp +144 +T}:T{ +.sp +2737 +T}:T{ +.sp +1100 +T} +T{ +.sp +100,000,000 +T}:T{ +.sp +10 T}:T{ .sp -1008 +144 T}:T{ .sp -7\&.6660 +27378 T}:T{ .sp -1\&.0 +1100 +T} +T{ +.sp +100,000,000 +T}:T{ +.sp +100 +T}:T{ +.sp +144 +T}:T{ +.sp +273785 +T}:T{ +.sp +1100 +T} +T{ +.sp +100,000,000 +T}:T{ +.sp +1000 +T}:T{ +.sp +144 +T}:T{ +.sp +2737850 +T}:T{ +.sp +1100 T} .TE .sp 1 .SH "RECOMMENDED RISKFACTOR VALUES" .sp -0\&.001 is a value for tie\-breaking in favor of shorter routes, but not really costing in any risk\&. +The default \fIfuzz\fR factor is 5%, so as you can see from the table above, that tends to overwhelm the effect of \fIriskfactor\fR less than about 5\&. .sp 1 is a conservative value for a stable lightning network with very few failures\&. .sp 1000 is an aggressive value for trying to minimize timeouts at all costs\&. +.sp +The default for lightning\-pay(7) is 10, which starts to become a major factor for larger amounts, and is basically ignored for tiny ones\&. .SH "RETURN VALUE" .sp On success, a "route" array is returned\&. Each array element contains \fIid\fR (the node being routed through), \fImsatoshi\fR (the millisatoshis sent), and \fIdelay\fR (the number of blocks to timeout at this node)\&. diff --git a/doc/lightning-getroute.7.txt b/doc/lightning-getroute.7.txt index 388072f1488b..771b95ca5ba7 100644 --- a/doc/lightning-getroute.7.txt +++ b/doc/lightning-getroute.7.txt @@ -21,11 +21,10 @@ There are two considerations for how good a route is: how low the fees are, and how long your payment will get stuck if a node goes down during the process. The 'riskfactor' floating-point field controls this tradeoff; it is the annual cost of your funds being stuck (as a -percentage), multiplied by the percentage chance of each node failing. +percentage). -For example, if you thought there was a 1% chance that a node would -fail, and it would cost you 20% per annum if that happened, -'riskfactor' would be 20. +For example, if you thought the inconvenience of having funds stuck was +worth 20% per annum interest, 'riskfactor' would be 20. If you didn't care about risk, 'riskfactor' would be zero. @@ -46,45 +45,59 @@ for the purposes of comparing routes. The formula used is the following approximation: ---- -hop-risk = num-hops x per-hop-risk -timeout-cost = blocks-timeout x per-block-cost -risk-fee = amount x hop-risk x timeout-cost +risk-fee = amount x blocks-timeout x per-block-cost ---- -We are given a 'riskfactor'; expressed as two multiplied percentages -is the same as fractions multiplied by 10000. There are 52596 blocks -per year, thus 'per-block-cost' x 'per-hop-risk' is riskfactor' -divided by 5,259,600,000. +We are given a 'riskfactor' expressed as a percentage. There are 52596 blocks +per year, thus 'per-block-cost' is 'riskfactor' divided by 5,259,600. The final result is: ---- -risk-fee = amount x num-hops x blocks-timeout x riskfactor / 5259600000 +risk-fee = amount x blocks-timeout x riskfactor / 5259600 ---- -Here are the risk fees as a percentage of the amount sent, using -various parameters. For comparison with actual fees, we assume nodes -charge 0.05%: +Here are the risk fees in millisatoshis, using various parameters. I +assume a channel charges the default of 1000 millisatoshis plus 1 +part-per-million. Common delay values on the network at 14 and 144. [options="header"] |======================= -|Riskfactor |Nodes | Delay per node |Risk Fee % |Route fee % -|0.001 |5 | 6 |0 |0.25 -|1 |5 | 6 |0 |0.25 -|1000 |5 | 6 |0.0029 |0.25 - -|0.001 |10 | 72 |0 |0.5 -|1 |10 | 72 |0.0001 |0.5 -|1000 |10 | 72 |0.1369 |0.5 - -|0.001 |20 | 1008 |0 |1.0 -|1 |20 | 1008 |0.0077 |1.0 -|1000 |20 | 1008 |7.6660 |1.0 +|Amount (msat) |Riskfactor | Delay |Risk Fee |Route fee +|10,000 |1 | 14 |0 |1001 +|10,000 |10 | 14 |0 |1001 +|10,000 |100 | 14 |2 |1001 +|10,000 |1000 | 14 |26 |1001 + +|1,000,000 |1 | 14 |2 |1001 +|1,000,000 |10 | 14 |26 |1001 +|1,000,000 |100 | 14 |266 |1001 +|1,000,000 |1000 | 14 |2661 |1001 + +|100,000,000 |1 | 14 |266 |1100 +|100,000,000 |10 | 14 |2661 |1100 +|100,000,000 |100 | 14 |26617 |1100 +|100,000,000 |1000 | 14 |266179 |1100 + +|10,000 |1 | 144 |0 |1001 +|10,000 |10 | 144 |2 |1001 +|10,000 |100 | 144 |27 |1001 +|10,000 |1000 | 144 |273 |1001 + +|1,000,000 |1 | 144 |27 |1001 +|1,000,000 |10 | 144 |273 |1001 +|1,000,000 |100 | 144 |2737 |1001 +|1,000,000 |1000 | 144 |27378 |1001 + +|100,000,000 |1 | 144 |2737 |1100 +|100,000,000 |10 | 144 |27378 |1100 +|100,000,000 |100 | 144 |273785 |1100 +|100,000,000 |1000 | 144 |2737850 |1100 |======================= RECOMMENDED RISKFACTOR VALUES ----------------------------- -0.001 is a value for tie-breaking in favor of shorter routes, but not really -costing in any risk. +The default 'fuzz' factor is 5%, so as you can see from the table above, +that tends to overwhelm the effect of 'riskfactor' less than about 5. 1 is a conservative value for a stable lightning network with very few failures. @@ -92,6 +105,9 @@ failures. 1000 is an aggressive value for trying to minimize timeouts at all costs. +The default for lightning-pay(7) is 10, which starts to become a major +factor for larger amounts, and is basically ignored for tiny ones. + RETURN VALUE ------------ diff --git a/doc/lightning-pay.7 b/doc/lightning-pay.7 index 1983dbab3a88..b4193205bf45 100644 --- a/doc/lightning-pay.7 +++ b/doc/lightning-pay.7 @@ -2,12 +2,12 @@ .\" Title: lightning-pay .\" Author: [see the "AUTHOR" section] .\" Generator: DocBook XSL Stylesheets v1.79.1 -.\" Date: 11/26/2018 +.\" Date: 02/01/2019 .\" Manual: \ \& .\" Source: \ \& .\" Language: English .\" -.TH "LIGHTNING\-PAY" "7" "11/26/2018" "\ \&" "\ \&" +.TH "LIGHTNING\-PAY" "7" "02/01/2019" "\ \&" "\ \&" .\" ----------------------------------------------------------------- .\" * Define some portability stuff .\" ----------------------------------------------------------------- @@ -34,7 +34,7 @@ lightning-pay \- Command for sending a payment to a BOLT11 invoice \fBpay\fR \fIbolt11\fR [\fImsatoshi\fR] [\fIdescription\fR] [\fIriskfactor\fR] [\fImaxfeepercent\fR] [\fIretry_for\fR] [\fImaxdelay\fR] [\fIexemptfee\fR] .SH "DESCRIPTION" .sp -The \fBpay\fR RPC command attempts to find a route to the given destination, and send the funds it asks for\&. If the \fIbolt11\fR does not contain an amount, \fImsatoshi\fR is required, otherwise if it is specified it must be \fInull\fR\&. If \fIbolt11\fR contains a description hash (\fIh\fR field) \fIdescription\fR is required, otherwise it is unused\&. The \fIriskfactor\fR is described in detail in lightning\-getroute(7), and defaults to 1\&.0\&. The \fImaxfeepercent\fR limits the money paid in fees, and defaults to 0\&.5\&. The maxfeepercent\*(Aq is a percentage of the amount that is to be paid\&. The `exemptfee option can be used for tiny payments which would be dominated by the fee leveraged by forwarding nodes\&. Setting exemptfee allows the maxfeepercent check to be skipped on fees that are smaller than exemptfee (default: 5000 millisatoshi)\&. +The \fBpay\fR RPC command attempts to find a route to the given destination, and send the funds it asks for\&. If the \fIbolt11\fR does not contain an amount, \fImsatoshi\fR is required, otherwise if it is specified it must be \fInull\fR\&. If \fIbolt11\fR contains a description hash (\fIh\fR field) \fIdescription\fR is required, otherwise it is unused\&. The \fIriskfactor\fR is described in detail in lightning\-getroute(7), and defaults to 10\&. The \fImaxfeepercent\fR limits the money paid in fees, and defaults to 0\&.5\&. The maxfeepercent\*(Aq is a percentage of the amount that is to be paid\&. The `exemptfee option can be used for tiny payments which would be dominated by the fee leveraged by forwarding nodes\&. Setting exemptfee allows the maxfeepercent check to be skipped on fees that are smaller than exemptfee (default: 5000 millisatoshi)\&. .sp The response will occur when the payment fails or succeeds\&. Once a payment has succeeded, calls to \fBpay\fR with the same \fIbolt11\fR will succeed immediately\&. .sp diff --git a/doc/lightning-pay.7.txt b/doc/lightning-pay.7.txt index 04e830f6517d..fab59a867d87 100644 --- a/doc/lightning-pay.7.txt +++ b/doc/lightning-pay.7.txt @@ -18,7 +18,7 @@ and send the funds it asks for. If the 'bolt11' does not contain an amount, 'msatoshi' is required, otherwise if it is specified it must be 'null'. If 'bolt11' contains a description hash ('h' field) 'description' is required, otherwise it is unused. The 'riskfactor' is described in detail -in lightning-getroute(7), and defaults to 1.0. +in lightning-getroute(7), and defaults to 10. The 'maxfeepercent' limits the money paid in fees, and defaults to 0.5. The `maxfeepercent' is a percentage of the amount that is to be paid. diff --git a/gossipd/gossip_wire.csv b/gossipd/gossip_wire.csv index 658f54681390..e86f1260ec83 100644 --- a/gossipd/gossip_wire.csv +++ b/gossipd/gossip_wire.csv @@ -29,7 +29,8 @@ gossip_getroute_request,3006 gossip_getroute_request,,source,struct pubkey gossip_getroute_request,,destination,struct pubkey gossip_getroute_request,,msatoshi,u64 -gossip_getroute_request,,riskfactor,u16 +# We don't pass doubles, so pass riskfactor * 1000000. +gossip_getroute_request,,riskfactor_by_million,u64 gossip_getroute_request,,final_cltv,u32 gossip_getroute_request,,fuzz,double gossip_getroute_request,,num_excluded,u16 diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index 9289f6a579bf..556e12aedd3f 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -1894,7 +1894,7 @@ static struct io_plan *getroute_req(struct io_conn *conn, struct daemon *daemon, struct pubkey source, destination; u64 msatoshi; u32 final_cltv; - u16 riskfactor; + u64 riskfactor_by_million; u32 max_hops; u8 *out; struct route_hop *hops; @@ -1909,7 +1909,7 @@ static struct io_plan *getroute_req(struct io_conn *conn, struct daemon *daemon, * avoid being too predictable. */ if (!fromwire_gossip_getroute_request(msg, msg, &source, &destination, - &msatoshi, &riskfactor, + &msatoshi, &riskfactor_by_million, &final_cltv, &fuzz, &excluded, &max_hops)) @@ -1921,8 +1921,8 @@ static struct io_plan *getroute_req(struct io_conn *conn, struct daemon *daemon, /* routing.c does all the hard work; can return NULL. */ hops = get_route(tmpctx, daemon->rstate, &source, &destination, - msatoshi, riskfactor, final_cltv, - fuzz, siphash_seed(), excluded, max_hops); + msatoshi, riskfactor_by_million / 1000000.0, final_cltv, + fuzz, pseudorand_u64(), excluded, max_hops); out = towire_gossip_getroute_reply(NULL, hops); daemon_conn_send(daemon->master, take(out)); diff --git a/gossipd/routing.c b/gossipd/routing.c index c9ce2ad99f96..e77c2e606359 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -529,9 +529,13 @@ find_route(const tal_t *ctx, struct routing_state *rstate, best = 0; for (i = 1; i <= max_hops; i++) { - if (dst->bfg[i].total < dst->bfg[best].total) + status_trace("%i hop solution: %"PRIu64" + %"PRIu64, + i, dst->bfg[i].total, dst->bfg[i].risk); + if (dst->bfg[i].total + dst->bfg[i].risk + < dst->bfg[best].total + dst->bfg[best].risk) best = i; } + status_trace("=> chose %i hop solution", best); /* No route? */ if (dst->bfg[best].total >= INFINITE) { @@ -1496,7 +1500,7 @@ struct route_hop *get_route(const tal_t *ctx, struct routing_state *rstate, const struct pubkey *destination, const u64 msatoshi, double riskfactor, u32 final_cltv, - double fuzz, const struct siphash_seed *base_seed, + double fuzz, u64 seed, const struct short_channel_id_dir *excluded, size_t max_hops) { @@ -1507,9 +1511,12 @@ struct route_hop *get_route(const tal_t *ctx, struct routing_state *rstate, struct route_hop *hops; struct node *n; u64 *saved_capacity; + struct siphash_seed base_seed; saved_capacity = tal_arr(tmpctx, u64, tal_count(excluded)); + base_seed.u.u64[0] = base_seed.u.u64[1] = seed; + /* Temporarily set excluded channels' capacity to zero. */ for (size_t i = 0; i < tal_count(excluded); i++) { struct chan *chan = get_channel(rstate, &excluded[i].scid); @@ -1521,8 +1528,8 @@ struct route_hop *get_route(const tal_t *ctx, struct routing_state *rstate, } route = find_route(ctx, rstate, source, destination, msatoshi, - riskfactor / BLOCKS_PER_YEAR / 10000, - fuzz, base_seed, max_hops, &fee); + riskfactor / BLOCKS_PER_YEAR / 100, + fuzz, &base_seed, max_hops, &fee); /* Now restore the capacity. */ for (size_t i = 0; i < tal_count(excluded); i++) { diff --git a/gossipd/routing.h b/gossipd/routing.h index 80c762243607..038a9ea5a0de 100644 --- a/gossipd/routing.h +++ b/gossipd/routing.h @@ -264,7 +264,7 @@ struct route_hop *get_route(const tal_t *ctx, struct routing_state *rstate, const u64 msatoshi, double riskfactor, u32 final_cltv, double fuzz, - const struct siphash_seed *base_seed, + u64 seed, const struct short_channel_id_dir *excluded, size_t max_hops); /* Disable channel(s) based on the given routing failure. */ diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index 3fc0a18ffaee..1b2a9b30fb54 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -352,7 +352,8 @@ static struct command_result *json_getroute(struct command *cmd, } u8 *req = towire_gossip_getroute_request(cmd, source, destination, - *msatoshi, *riskfactor * 1000, + *msatoshi, + *riskfactor * 1000000.0, *cltv, fuzz, excluded, *max_hops); diff --git a/plugins/pay.c b/plugins/pay.c index 19905d9216cf..352c25f7cdb7 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -92,7 +92,10 @@ struct pay_command { /* Channels which have failed us. */ const char **excludes; - /* Any routehints to use. */ + /* Current routehint, if any. */ + struct route_info *current_routehint; + + /* Any remaining routehints to try. */ struct route_info **routehints; /* Current node during shadow route calculation. */ @@ -179,12 +182,23 @@ static struct command_result *waitsendpay_expired(struct command *cmd, return command_done_err(cmd, PAY_STOPPED_RETRYING, errmsg, data); } -/* Try again with the next routehint (or none if that was the last) */ static struct command_result *next_routehint(struct command *cmd, struct pay_command *pc) { - tal_arr_remove(&pc->routehints, 0); - return start_pay_attempt(cmd, pc, "Removed route hint"); + if (tal_count(pc->routehints) > 0) { + pc->current_routehint = pc->routehints[0]; + tal_arr_remove(&pc->routehints, 0); + return start_pay_attempt(cmd, pc, "Trying route hint"); + } + + /* No (more) routehints; we're out of routes. */ + /* If we eliminated one because it was too pricy, return that. */ + if (pc->expensive_route) + return command_fail(cmd, PAY_ROUTE_TOO_EXPENSIVE, + "%s", pc->expensive_route); + + return command_fail(cmd, PAY_ROUTE_NOT_FOUND, + "Could not find a route"); } static struct command_result *waitsendpay_error(struct command *cmd, @@ -222,9 +236,8 @@ static struct command_result *waitsendpay_error(struct command *cmd, return waitsendpay_expired(cmd, pc); } - /* If failure is in routehint part, eliminate that */ - if (tal_count(pc->routehints) != 0 - && channel_in_routehint(pc->routehints[0], buf, scidtok)) + /* If failure is in routehint part, try next one */ + if (channel_in_routehint(pc->current_routehint, buf, scidtok)) return next_routehint(cmd, pc); /* Otherwise, add erring channel to exclusion list. */ @@ -370,8 +383,7 @@ static bool maybe_exclude(struct pay_command *pc, scid = json_get_member(buf, route, "channel"); - if (tal_count(pc->routehints) != 0 - && channel_in_routehint(pc->routehints[0], buf, scid)) + if (channel_in_routehint(pc->current_routehint, buf, scid)) return false; dir = json_get_member(buf, route, "direction"); @@ -399,9 +411,9 @@ static struct command_result *getroute_done(struct command *cmd, plugin_err("getroute gave no 'route'? '%.*s'", result->end - result->start, buf); - if (tal_count(pc->routehints)) + if (pc->current_routehint) attempt->route = join_routehint(pc->ps->attempts, buf, t, - pc, pc->routehints[0]); + pc, pc->current_routehint); else attempt->route = json_strdup(pc->ps->attempts, buf, t); @@ -441,11 +453,7 @@ static struct command_result *getroute_done(struct command *cmd, pc->excludes[tal_count(pc->excludes)-1]); } - if (tal_count(pc->routehints) != 0) - return next_routehint(cmd, pc); - - return command_fail(cmd, PAY_ROUTE_TOO_EXPENSIVE, - "%s", pc->expensive_route); + return next_routehint(cmd, pc); } if (delay > pc->maxdelay) { @@ -472,11 +480,7 @@ static struct command_result *getroute_done(struct command *cmd, pc->excludes[tal_count(pc->excludes)-1]); } - if (tal_count(pc->routehints) != 0) - return next_routehint(cmd, pc); - - return command_fail(cmd, PAY_ROUTE_TOO_EXPENSIVE, - "%s", pc->expensive_route); + return next_routehint(cmd, pc); } if (pc->desc) @@ -497,18 +501,21 @@ static struct command_result *getroute_error(struct command *cmd, const jsmntok_t *error, struct pay_command *pc) { + int code; + const jsmntok_t *codetok; + attempt_failed_tok(pc, "getroute", buf, error); - /* If we were trying to use a routehint, remove and try again. */ - if (tal_count(pc->routehints) != 0) - return next_routehint(cmd, pc); + codetok = json_get_member(buf, error, "code"); + if (!json_to_int(buf, codetok, &code)) + plugin_err("getroute error gave no 'code'? '%.*s'", + error->end - error->start, buf + error->start); - /* If we've run out of routes, there might be a good reason. */ - if (pc->expensive_route) - return command_fail(cmd, PAY_ROUTE_TOO_EXPENSIVE, - "%s", pc->expensive_route); + /* Strange errors from getroute should be forwarded. */ + if (code != PAY_ROUTE_NOT_FOUND) + return forward_error(cmd, buf, error, pc); - return forward_error(cmd, buf, error, pc); + return next_routehint(cmd, pc); } /* Deep copy of excludes array. */ @@ -559,17 +566,17 @@ static struct command_result *start_pay_attempt(struct command *cmd, /* If we have a routehint, try that first; we need to do extra * checks that it meets our criteria though. */ - if (tal_count(pc->routehints)) { + if (pc->current_routehint) { amount = route_msatoshi(pc->msatoshi, - pc->routehints[0], - tal_count(pc->routehints[0])); + pc->current_routehint, + tal_count(pc->current_routehint)); dest = type_to_string(tmpctx, struct pubkey, - &pc->routehints[0][0].pubkey); - max_hops -= tal_count(pc->routehints[0]); + &pc->current_routehint[0].pubkey); + max_hops -= tal_count(pc->current_routehint); cltv = route_cltv(pc->final_cltv, - pc->routehints[0], - tal_count(pc->routehints[0])); - attempt.routehint = tal_steal(pc->ps, pc->routehints[0]); + pc->current_routehint, + tal_count(pc->current_routehint)); + attempt.routehint = tal_steal(pc->ps, pc->current_routehint); } else { amount = pc->msatoshi; dest = pc->dest; @@ -826,7 +833,7 @@ static struct command_result *handle_pay(struct command *cmd, p_req("bolt11", param_string, &b11str), p_opt("msatoshi", param_u64, &msatoshi), p_opt("description", param_string, &pc->desc), - p_opt_def("riskfactor", param_double, &riskfactor, 1.0), + p_opt_def("riskfactor", param_double, &riskfactor, 10), p_opt_def("maxfeepercent", param_percent, &maxfeepercent, 0.5), p_opt_def("retry_for", param_number, &retryfor, 60), p_opt_def("maxdelay", param_number, &maxdelay, @@ -871,6 +878,8 @@ static struct command_result *handle_pay(struct command *cmd, pc->stoptime = timeabs_add(time_now(), time_from_sec(*retryfor)); pc->excludes = tal_arr(cmd, const char *, 0); pc->ps = add_pay_status(pc, b11str); + /* We try first without using routehint */ + pc->current_routehint = NULL; pc->routehints = filter_routehints(pc, b11->routes); pc->expensive_route = NULL; diff --git a/tests/test_pay.py b/tests/test_pay.py index 3c62627856af..1dea06e04bcc 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -76,10 +76,14 @@ def test_pay_limits(node_factory): # It should have retried (once without routehint, too) status = l1.rpc.call('paystatus', {'bolt11': inv['bolt11']})['pay'][0]['attempts'] - assert len(status) == 3 + + # Hits weird corner case: it excludes channel, then uses routehint + # which reintroduces it, so then it excludes other channel. + assert len(status) == 4 assert status[0]['strategy'] == "Initial attempt" assert status[1]['strategy'].startswith("Excluded expensive channel ") - assert status[2]['strategy'] == "Removed route hint" + assert status[2]['strategy'] == "Trying route hint" + assert status[3]['strategy'].startswith("Excluded expensive channel ") # Delay too high. with pytest.raises(RpcError, match=r'Route wanted delay of .* blocks') as err: @@ -88,10 +92,11 @@ def test_pay_limits(node_factory): assert err.value.error['code'] == PAY_ROUTE_TOO_EXPENSIVE # Should also have retried. status = l1.rpc.call('paystatus', {'bolt11': inv['bolt11']})['pay'][1]['attempts'] - assert len(status) == 3 + assert len(status) == 4 assert status[0]['strategy'] == "Initial attempt" assert status[1]['strategy'].startswith("Excluded delaying channel ") - assert status[2]['strategy'] == "Removed route hint" + assert status[2]['strategy'] == "Trying route hint" + assert status[3]['strategy'].startswith("Excluded delaying channel ") # This works, because fee is less than exemptfee. l1.rpc.call('pay', {'bolt11': inv['bolt11'], 'msatoshi': 100000, 'maxfeepercent': 0.0001, 'exemptfee': 2000}) @@ -1259,13 +1264,21 @@ def test_pay_routeboost(node_factory, bitcoind): assert 'description' not in only_one(status['pay']) assert 'routehint_modifications' not in only_one(status['pay']) assert 'local_exclusions' not in only_one(status['pay']) - attempt = only_one(only_one(status['pay'])['attempts']) - assert attempt['age_in_seconds'] <= time.time() - start - assert attempt['duration_in_seconds'] <= end - start - assert only_one(attempt['routehint']) - assert only_one(attempt['routehint'])['id'] == l3.info['id'] - assert only_one(attempt['routehint'])['msatoshi'] == 10**5 + 1 + 10**5 // 100000 - assert only_one(attempt['routehint'])['delay'] == 5 + 6 + # First attempt will fail, then it will try route hint + attempts = only_one(status['pay'])['attempts'] + assert len(attempts) == 2 + assert attempts[0]['strategy'] == "Initial attempt" + # FIXME! + PAY_ROUTE_NOT_FOUND = 205 + assert attempts[0]['failure']['code'] == PAY_ROUTE_NOT_FOUND + assert attempts[1]['strategy'] == "Trying route hint" + assert 'success' in attempts[1] + assert attempts[1]['age_in_seconds'] <= time.time() - start + assert attempts[1]['duration_in_seconds'] <= end - start + assert only_one(attempts[1]['routehint']) + assert only_one(attempts[1]['routehint'])['id'] == l3.info['id'] + assert only_one(attempts[1]['routehint'])['msatoshi'] == 10**5 + 1 + 10**5 // 100000 + assert only_one(attempts[1]['routehint'])['delay'] == 5 + 6 # With dev-route option we can test longer routehints. if DEVELOPER: @@ -1287,43 +1300,11 @@ def test_pay_routeboost(node_factory, bitcoind): 'dev-routes': [routel3l4l5]}) l1.rpc.pay(inv['bolt11']) status = l1.rpc.call('paystatus', [inv['bolt11']]) - assert len(only_one(status['pay'])['attempts']) == 1 - assert 'failure' not in only_one(status['pay'])['attempts'][0] - assert 'success' in only_one(status['pay'])['attempts'][0] - - # Now test that it falls back correctly to not using routeboost - # if it can't route to the node mentioned - routel4l3 = [{'id': l4.info['id'], - 'short_channel_id': scid34, - 'fee_base_msat': 1000, - 'fee_proportional_millionths': 10, - 'cltv_expiry_delta': 6}] - inv = l3.rpc.call('invoice', {'msatoshi': 10**5, - 'label': 'test_pay_routeboost3', - 'description': 'test_pay_routeboost3', - 'dev-routes': [routel4l3]}) - l1.rpc.pay(inv['bolt11']) - status = l1.rpc.call('paystatus', [inv['bolt11']]) assert len(only_one(status['pay'])['attempts']) == 2 assert 'failure' in only_one(status['pay'])['attempts'][0] assert 'success' not in only_one(status['pay'])['attempts'][0] - routehint = only_one(status['pay'])['attempts'][0]['routehint'] - assert [h['channel'] for h in routehint] == [r['short_channel_id'] for r in routel4l3] assert 'failure' not in only_one(status['pay'])['attempts'][1] assert 'success' in only_one(status['pay'])['attempts'][1] - assert 'routehint' not in only_one(status['pay'])['attempts'][1] - - # Similarly if it can route, but payment fails. - routel2bad = [{'id': l2.info['id'], - 'short_channel_id': scid34, # Invalid scid - 'fee_base_msat': 1000, - 'fee_proportional_millionths': 10, - 'cltv_expiry_delta': 6}] - inv = l3.rpc.call('invoice', {'msatoshi': 10**5, - 'label': 'test_pay_routeboost4', - 'description': 'test_pay_routeboost4', - 'dev-routes': [routel2bad]}) - l1.rpc.pay(inv['bolt11']) # Finally, it should fall back to second routehint if first fails. # (Note, this is not public because it's not 6 deep) @@ -1350,10 +1331,61 @@ def test_pay_routeboost(node_factory, bitcoind): assert 'local_exclusions' not in only_one(status['pay']) attempts = only_one(status['pay'])['attempts'] - # First failed, second succeeded. - assert len(attempts) == 2 + # First two failed (w/o routehint and w bad hint), third succeeded. + assert len(attempts) == 3 assert 'success' not in attempts[0] - assert 'success' in attempts[1] + assert 'success' not in attempts[1] + assert 'success' in attempts[2] + + assert [h['channel'] for h in attempts[1]['routehint']] == [r['short_channel_id'] for r in routel3l4l5] + assert [h['channel'] for h in attempts[2]['routehint']] == [r['short_channel_id'] for r in routel3l5] + + +def test_pay_direct(node_factory, bitcoind): + """Check that we prefer the direct route. + """ + # l2->l3 is really cheap by comparison. + l0, l1, l2, l3 = node_factory.get_nodes(4, opts=[{'fee-base': 1000, + 'cltv-delta': 14}, + {'fee-base': 1000, + 'cltv-delta': 14}, + {'fee-base': 0, + 'cltv-delta': 14}, + {'fee-base': 1000, + 'cltv-delta': 14}]) + + # Direct channel l0->l1->l3 + l0.rpc.connect(l1.info['id'], 'localhost', l1.port) + # Waiting takes a *long* time if !DEVELOPER. + l0.fund_channel(l1, 10**7, wait_for_active=False) + l1.rpc.connect(l3.info['id'], 'localhost', l3.port) + l1.fund_channel(l3, 10**7, wait_for_active=False) + # Indirect route l0->l1->l2->l3 + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + l1.fund_channel(l2, 10**7, wait_for_active=False) + l2.rpc.connect(l3.info['id'], 'localhost', l3.port) + c3 = l2.fund_channel(l3, 10**7, wait_for_active=False) + + # Let channels lock in. + bitcoind.generate_block(5) + + # Make sure l0 knows the l2->l3 channel. + # Without DEVELOPER, channel lockin can take 30 seconds to detect, + # and gossip 2 minutes to propagate + wait_for(lambda: l0.is_channel_active(c3), timeout=180) + + # Find out how much msatoshi l1 owns on l1->l2 channel. + l1l2msatreference = only_one(l1.rpc.getpeer(l2.info['id'])['channels'])['msatoshi_to_us'] + + # Try multiple times to ensure that route randomization + # will not override our preference for direct route. + for i in range(8): + inv = l3.rpc.invoice(15000000, 'pay{}'.format(i), 'desc')['bolt11'] + + l0.rpc.pay(inv) - assert [h['channel'] for h in attempts[0]['routehint']] == [r['short_channel_id'] for r in routel3l4l5] - assert [h['channel'] for h in attempts[1]['routehint']] == [r['short_channel_id'] for r in routel3l5] + # We should have gone the direct route, so + # l1->l2 channel msatoshi_to_us should not + # have changed. + l1l2msat = only_one(l1.rpc.getpeer(l2.info['id'])['channels'])['msatoshi_to_us'] + assert l1l2msat == l1l2msatreference