Commit Graph

347 Commits

Author SHA1 Message Date
Rusty Russell 0c7d04bd98 libplugin-pay: fix default CLTV.
This was changed recently, but without a bolt quote, we didn't find
this.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-09-02 06:59:13 +09:30
Rusty Russell 1746406e41 Makefile: normalize all the Makefiles
We create ALL_PROGRAMS, ALL_TEST_PROGRAMS, ALL_C_SOURCES and
ALL_C_HEADERS.  Then the toplevel Makefile knows which are
autogenerated (by wildcard), so it can have all the rules to clean
them or check the source as necessary.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-31 21:33:26 -05:00
Rusty Russell 8150d28575 Makefile: use generic rules to make spec-derived sources.
Now we use the same Makefile rules for all CSV->C generation.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-31 21:33:26 -05:00
ZmnSCPxj jxPCSnmZ 932709cad9 plugins/libplugin-pay.c: Micro-optimize start_block sampling.
Using `waitblockheight 0` is a very slightly faster query than `getinfo`.
Also, avoid querying blockheight for child payments (allow `waitblockheight`
paymod to provide the blockheight returned from the `waitblockheight`, and
just resample the starting blockheight from the parent).

Changelog-None: pointless micro-optimization
2020-08-28 16:40:27 +02:00
ZmnSCPxj jxPCSnmZ 05daa8e5f3 plugins/libplugin-pay.c: Micro-optimization of plugin_is_finished.
This was checked with `gcc -S -O2` to see how an optimized build
would compile the function.
The original code completed calls into each child (and the `.s`
file showed that GCC 9.x was not smart enough to do early-out).

This modification explicitly does early-out, and avoids call-return
stack overhead for the common case where a payment is an ancestor
of a long line of single-child payments due to retrying.

Changelog-None: pointless micro-optimization
2020-08-28 16:40:27 +02:00
Rusty Russell 496c0dd1e6 common/random_select: central place for reservoir sampling.
Turns out we can make quite a simple API out of it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-28 10:56:50 +09:30
Rusty Russell 12d0d5c185 amount: cleanup usage.
We've got some recently-added primitives which help.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-27 18:16:28 +02:00
Christian Decker 21d87f7075 pay: Implement simple presplit fix for ludicrous amounts
This is the simplest possible fix: increase the target amount until we get
the desired number of parts, while still bucketizing payments together that
are in approximately the same size.

The current logic puts all payments that are in the range x < amount <= 16*x
in the same bucket, making them harder to distinguish.

Changelog-Fixed: pay: The `presplit` modifier now supports large payments without exhausting the available HTLCs.
2020-08-27 10:19:21 +09:30
ZmnSCPxj jxPCSnmZ 128adf0938 plugins/libplugin-pay.c: Round-robin routehints when splitting.
This improves the success rate of `test_mpp_interference_2`, though
still not quite up to the level that we can remove `@flaky` from it.
2020-08-26 09:29:46 +09:30
ZmnSCPxj jxPCSnmZ c27d7a3110 plugins/libplugin-pay.c: Store the route description, and re-report on failure. 2020-08-25 12:17:18 +02:00
ZmnSCPxj jxPCSnmZ 6468616c02 plugins/libplugin-pay.c: Propagate local_id from parent to child payment object. 2020-08-25 12:17:18 +02:00
ZmnSCPxj jxPCSnmZ d8678467fa plugins/libplugin-pay.c: Show routes being tried, also print updates to channel hints not just initial creations. 2020-08-25 12:17:18 +02:00
ZmnSCPxj jxPCSnmZ d89c77c0ce plugins/libplugin-pay.c: Describe the bits of unrecognized failure codes. 2020-08-25 12:17:18 +02:00
ZmnSCPxj jxPCSnmZ 0d2d85ab5f plugins/libplugin-pay.c: Also print events that create new sub-payments. 2020-08-25 12:17:18 +02:00
ZmnSCPxj jxPCSnmZ 98583e84b5 plugins/libplugin-pay.c: Give cmd id and partid for each log message.
Changelog-None: internal debugging

Makes it easier to debug payments with tons of splits.
2020-08-25 12:17:18 +02:00
Rusty Russell 28884864d0 plugins/pay: don't bother parsing nodeid for summary.
We have sanity checks in there that it's a valid point.  Simply store
the JSON token like we do with others.

time lightning-cli -R --network=regtest --lightning-dir /tmp/ltests-k8jhvtty/test_pay_stress_1/lightning-1/ listpays > /dev/null

Before:
	real	0m2.054s
	user	0m0.114s
	sys	0m0.024s

After:
	real	0m1.781s
	user	0m0.127s
	sys	0m0.013s

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-21 09:52:33 +09:30
Rusty Russell 4ba3797923 plugins/pay: iterate the htable properly, not from start each time.
time lightning-cli -R --network=regtest --lightning-dir /tmp/ltests-k8jhvtty/test_pay_stress_1/lightning-1/ listpays > /dev/null

Before:
	real	0m12.447s
	user	0m0.143s
	sys	0m0.008s

After:
	real	0m2.054s
	user	0m0.114s
	sys	0m0.024s

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-21 09:52:33 +09:30
Rusty Russell 05636e367e libplugin: do partial parsing instead of memmem hack.
memmem is also O(n^2), though it's faster.  Now we have infrastructure,
let's do incremental parsing.

time lightning-cli -R --network=regtest --lightning-dir /tmp/ltests-k8jhvtty/test_pay_stress_1/lightning-1/ listpays > /dev/null

Before:
	real	0m13.674s
	user	0m0.131s
	sys	0m0.024s

After:
	real	0m12.447s
	user	0m0.143s
	sys	0m0.008s

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-21 09:52:33 +09:30
Rusty Russell 628ae0a15b libplugin: do incremental parsing on lightningd commands.
This doesn't make any difference, since lightningd generally sends us
short commands (command responses are via the rpc loop, which is
already done), but it's harmless.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-21 09:52:33 +09:30
Rusty Russell 3ed03f9c8f common: make json_parse_input API retry friendly.
The jsmn parser is a beautiful piece of code.  In particular, you can parse
part of a string, then continue where you left off.

We don't take advantage of this, however, meaning for large JSON objects
we parse them multiple times before finally having enough to complete.

Expose the parser state and tokens through the API, so the caller can pass
them in repeatedly.  For the moment, every caller is allocates each time
(except the unit tests).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-21 09:52:33 +09:30
Rusty Russell f2d0a1d406 plugins/bcli: use simplified parser, unify bad JSON paths.
They should all show the complete JSON, so unify them.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-21 09:52:33 +09:30
Rusty Russell 80b2e1e298 common: add simple json parse wrapper for the complete cases.
We're going to change the API on the more complete JSON parser, so
make and use a simple API for the easy cases.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-21 09:52:33 +09:30
Rusty Russell 40f17d994c libplugin: don't parse partial JSON repeatedly.
Tested on a test node which had made 50,000 payment, with no optimization.

For comparison, time for 'listsendpays' was 0.983s.

time lightning-cli -R --network=regtest --lightning-dir /tmp/ltests-k8jhvtty/test_pay_stress_1/lightning-1/ listpays > /dev/null

Before:
	real	0m52.415s
	user	0m0.127s
	sys	0m0.044s

After:
	real	0m42.741s
	user	0m0.149s
	sys	0m0.016s

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: libplugin: significant speedups for reading large JSON replies (e.g. calling listsendpays on large nodes, or listchannels / listnodes).
2020-08-21 09:52:33 +09:30
Rusty Russell 6b39731c6a fundchannel: provide outnum
It's currently always 0, but it won't be once we replace txprepare.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `fundchannel` has new `outnum` field indicating which output of the transaction funds the channel.
2020-08-13 12:37:02 -05:00
ZmnSCPxj jxPCSnmZ f81611e551 plugins/libplugin-pay.c: Make sure blockheight disagreement does not prevent all future progress.
Blockheight disagreement is signalled with a permanent failure at the
end node, but is actually a transient failure.
2020-08-13 12:50:16 +02:00
Christian Decker 8769f9ed93 pay: Fix final TLV payload if not going through MPP modifiers
Reported-by: ZmnSCPxj
Signed-off-by: Christian Decker <@cdecker>

Changelog-Fixed: pay: Correct a case where we put the sub-payment value instead of the *total* value in the `total_msat` field of a multi-part payment.
2020-08-13 12:50:16 +02:00
ZmnSCPxj jxPCSnmZ 0279be1d13 plugins/libplugin-pay.c: Be less aggressive with advancing through routehints.
Only advance through routehints if no route was found at all, or if the
estimated capacity at the routehint is lower than the amount that we
have to send through the routehint.

Changelog-Fixed: pay: Be less aggressive with forgetting routehints.
2020-08-13 12:50:16 +02:00
Christian Decker 3d902501e5 pay: Group payments by their payment_hash, not by bolt11 string
This is quicker and guaranteed to work even for payments that were not
initiated via an invoice.
2020-08-12 19:10:48 +02:00
Christian Decker 894c886bdd pay: Inherit payment label to all children 2020-08-12 19:10:48 +02:00
Christian Decker 0dcd974d97 pytest: Reproduce #3915 2020-08-12 19:10:48 +02:00
Rusty Russell fc097b8b48 libplugin: allow commands and options to mark themselves deprecated.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-11 08:43:18 +09:30
Rusty Russell c85b14f06d libplugin: initialize deprecated in getmanifest.
We currently do it by calling listconfigs, but that may be too late.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-11 08:43:18 +09:30
Vincenzo Palazzo 1521c29fcf listpays mod 1: add destination inside the response when bolt11 is null
Changelog-Added: JSON-RPC: `listpays` now lists the `destination` if it was provided (e.g., via the `pay` plugin or `keysend` plugin)
2020-08-09 16:03:03 +02:00
Vincent 700897f06e listpays: fixed bolt11 null with keysend and update doc command
listpays: make doc-all missed
Changelog-Added: JSON-RPC: `listpays` can be used to query payments using the `payment_hash`
Changelog-Added: JSON-RPC: `listpays` now includes the `payment_hash`
2020-08-09 16:03:03 +02:00
Christian Decker 723b7223b7 pay: Add timestamp of first part to `listpays`
Changelog-Added: JSON-RPC: The result returned by `listpays` now includes the timestamp of the first part of the payment
2020-08-08 12:53:12 +02:00
ZmnSCPxj jxPCSnmZ a5fc66c382 plugins/Makefile: Only link in libplugin-pay for plugins that need it.
Changelog-None: cleanup only.

Before this change:

```
$ ls -l plugins/bcli
-rwxrwxr-x 1 zmnscpxj zmnscpxj 1914976 Aug  5 21:54 plugins/bcli
```

After this change:

```
$ ls -l plugins/bcli
-rwxrwxr-x 1 zmnscpxj zmnscpxj 1830552 Aug  5 22:00 plugins/bcli
```

We already duplicate a lot of code between `lightningd` and eeach
builtin plugin because we do not use .so for `common/`, but including
an object file with code that is not referenced in the executable is
fairly low-hanging size optimization fruit.
2020-08-07 10:40:10 +09:30
ZmnSCPxj jxPCSnmZ 4fde45669c plugins/pay.c: Fix the routehints/presplit conflict.
Changelog-Fixed: pay: Fixed a bug where routehints would be ignored if the payment exceeded 10,000 satoshi. This is particularly bad if the payee is only reachable via routehints in an invoice.
2020-08-06 17:36:58 +02:00
Rusty Russell fa829f23db amount: add amount_msat_scale, amount_msat_ratio, amount_{msat,sat}_div
It's not all that rare to do these operations, and requiring annotations
for it is a little painful.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2020-08-06 09:36:47 +09:30
ZmnSCPxj jxPCSnmZ 094eac4e95 plugins/libplugin-pay.c: Properly handle exclusions for routehints with two hops or more.
Arguably a low-priority bug since no current node ever generates routehints longer
than one hop.

However, it is possible as an edge case, if the destination is directly accessible
*and* supports multiple channels, that we route through the destination, one of the
*other* channels it has not in the routehint, to the entry point, and then through
the routehint.

This change removes the risk of the above edge case.

Changelog-None: arguably a low-priority bug.
2020-08-03 15:20:29 +09:30
Antoine Poinsot 905730341a bcli: explicitly check at startup that bitcoind does relay transactions
We've had problems with blocksonly in the past and bitcoind still allows
to use outdated (thus potentially dangerous estimates) while running
bitcoin with -blocksonly.

ZmnSCPxj mentionned that we still don't document this, but i figured
that in this specific case an explicit check and error seems preferable.

Changelog-Added: We now explicitly check at startup that our default Bitcoin backend (bitcoind) does relay transactions.

Proposed-by: ZmnSCPxj <zmnscpxj@protonmail.com>
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
2020-08-03 12:46:37 +09:30
Antoine Poinsot 5be07c5fe3 bcli: poll `getnetworkinfo` at startup
This allows us to kill two birds with one stone: once connected, we use
the output of the successful call to do some sanity checks (only
checking the version for now, but more are yet to come!).

Changelog-Added: We now explicitly check at startup the version of our default Bitcoin backend (bitcoind).
Co-Authored-by: ZmnSCPxj <zmnscpxj@protonmail.com>
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
2020-08-03 12:46:37 +09:30
Christian Decker a456d08ad0 pay: Be less aggressive when estimating channel capacity
We'd previously take the failed attempt and estimate the failing channel's
capacity at 3/4 of the attempted amount, which is rather aggressive. This
reduces this aggressiveness to use the exact amount tried, but excluding on
equality. This still skips attempting the same route with the same amount, but
also permits attempts that are in the range [3/4, 1] of the failed attempt
amount to still be attempted.
2020-08-03 12:15:08 +09:30
Christian Decker 311594b2cd jsonrpc: Add amount received by recipient to `listpays` result
We sum up the amounts just like we do with `amount_sent`, however we may have
parts that don't have that annotation, so make it optional.

Suggested-by: Rusty Russell <@rustyrussell>
2020-07-28 16:17:39 +09:30
Christian Decker d7cca0781d jsonrpc: Add `msatoshi` argument to `sendonion` to annotate
While not directly necessary, it still feeds the `listpays` result, and so we
should pass it along if we can, so we don't have to rely solely on the
`amount_sent` field, which includes the fees.

Reported-by: Rusty Russell <@rustyrussell>
2020-07-28 16:17:39 +09:30
Vincent 81fd552e84 plugins/pay: hand bolt11 arg to sendonion if we have one (i.e. for `pay`)
[ Extracted into standalone patch and comment added by RR ]
2020-07-27 13:11:14 +02:00
Christian Decker 15d1a190a0 pay: Remove duplicate message field
jsonrpc_stream_fail already adds a message field.
2020-07-24 11:35:49 +02:00
Christian Decker 85ec438d34 paymod: Routehintmod signals that we can retry if getroute fails
The shortcut in the retry_mod that we can skip retrying if getroute fails or
we have no result is only valid if the parameters don't change. As we iterate
through the routehints the parameters change, and so we must signal to the
retry_mod that it can retry even in those cases.
2020-07-24 11:35:49 +02:00
Christian Decker 52a8b8f9e7 paymod: Update step before creating child payments
The child payments will sometimes depend on the step of the parent, and making
sure that the parent state is correct before we create the children is
therefore important.
2020-07-24 11:35:49 +02:00
Rusty Russell 2556df5f7c plugins/pay: Exclude the entrypoint to a routehint to avoid cycles
This uses @cdecker's idea of excluding the routehinted channel from the route,
and also consumes the route hints as it goes so that it makes progress.

I don't know if this is correct, but it reliably passes tests/test_pay.py::test_tlv_or_legacy
now.
2020-07-24 11:35:49 +02:00
Christian Decker 56dd18e01e paymod: Iterate through the routehints in order
We store an offset of the current routehint in the modifier data. It gets
incremented on retry, and it gets reset to 0 on split. This is because once we
split we have a different amount and a previously unusable routehint becomes
usable again.
2020-07-24 11:35:49 +02:00